added the api for trip details, created the tripdetailpane with all information #28
Conversation
aaronbrethorst
commented
Jul 25, 2024
•
aaronbrethorst
commented
- Left align back button and add button text on trip details (see screenshot)
- Use real data for the departure times shown on the right side of the pane (see screenshot)
- Fill in the current location of the bus, if available. See iOS equivalent code here: https://github.com/OneBusAway/onebusaway-ios/blob/main/OBAKit/Trip/TripStopListItem.swift#L73
- Fill in the user icon, too. iOS code here: https://github.com/OneBusAway/onebusaway-ios/blob/main/OBAKit/Trip/TripStopListItem.swift#L66
…e current location of bus (30 sec interval update), enhanced the modal pane UI, and user icon
src/components/oba/StopPane.svelte
Outdated
| /> | ||
| {/each} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| {/if} | ||
|
|
||
| {#if showTripDetails} | ||
| <div class="trip-details-modal scrollbar-hidden"> |
There was a problem hiding this comment.
Please extract this into a new component. TripDetailsModal, maybe?
| /> | ||
| {/if} | ||
| {#if index === currentStopIndex} | ||
| <FontAwesomeIcon |
There was a problem hiding this comment.
I'm not seeing this work on any of the stops I'm testing out. Can you double check it?
There was a problem hiding this comment.
Fixed the issue now.
|
|
||
| onMount(() => { | ||
| loadTripDetails(); | ||
| interval = setInterval(loadTripDetails, 30000); |
| url += `&serviceDate=${serviceDate}`; | ||
| } | ||
| const response = await fetch(url); | ||
| if (response.ok) { |
There was a problem hiding this comment.
try flipping these conditions around and using an early return in order to decrease the level of control flow nesting. i.e.:
if (!response.ok) {
error = 'Unable to fetch trip details';
return;
}less nesting improves readability and understanding for anyone who reads this code after you :)
There was a problem hiding this comment.
Done with the recommended changes.
| const response = await fetch(url); | ||
| if (response.ok) { | ||
| const data = await response.json(); | ||
| tripDetails = data.data.entry; |
There was a problem hiding this comment.
I suggest extracting data.data into its own local variable and using that in place of data.data throughout this code:
const jsonBody = await response.json();
const data = jsonBody.data;
There was a problem hiding this comment.
Extracted the data into the local variable.
| const data = await response.json(); | ||
| tripDetails = data.data.entry; | ||
|
|
||
| if (data.data.references && data.data.references.routes) { |
There was a problem hiding this comment.
now that you have data extracted above, you can use that along with optional chaining to simplify these conditions:
if (data?.references?.routes) {
// ...
}
| routeInfo = data.data.references.routes.find((route) => route.id === tripDetails.routeId); | ||
| } | ||
|
|
||
| if (data.data.references && data.data.references.stops) { |
| }, {}); | ||
| } | ||
|
|
||
| if (tripDetails.status && tripDetails.status.closestStop) { |
There was a problem hiding this comment.
and this becomes if (tripDetails.status?.closestStop)
|
|
||
| calculateBusPosition(); | ||
|
|
||
| console.log('Trip details:', tripDetails); |
There was a problem hiding this comment.
are these still necessary? if not, please delete them
There was a problem hiding this comment.
No! Deleted all logs.
tarunsinghofficial
force-pushed
the
trip-details
branch
from
b0e0051 to
49a385c
Compare
|
Hi @aaronbrethorst, This PR is now ready to be merged. The issue of not showing the user location is fixed now. Working fine. |
There was a problem hiding this comment.
nice work @tarunsinghofficial. I made a few more tweaks and am merging. thanks for your hard work on this!
* Replace the location dot with an icon of a person walking * Move it into the bottom right corner of the trip icon box so that both the bus icon and person icons can be seen at once
aaronbrethorst
force-pushed
the
trip-details
branch
from
12ec3c3 to
88883b3
Compare
Nice UI enhancement on Tripdetailspane. Thanks for the changes. |
