-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Added GlobalMessage to TripSearchScreen #4918
Conversation
@@ -404,6 +405,18 @@ export const Dashboard_TripSearchScreen: React.FC<RootProps> = ({ | |||
{t(TripSearchTexts.searchState.noResultReason.MissingLocation)} | |||
</ThemeText> | |||
)} | |||
{!!tripPatterns.length && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be shown based on loading state instead of length? We might want to show messages in cases where there are no results. Or can it be shown even if loading hasn't completed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good point. I thought tripPatterns.length
was a reasonable proxy for loading state, but you are right that there are cases where that is not the case. Will fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, if I use !isSearching
there is a delay after tripPatterns
has elements which leads to a little uncomfortable experience. See the videos. The second video is using tripPatterns.length
isSearching.mp4
tripPatterns-length.mp4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments after pair-review:
- Lets just show the box regardless of searching state, as suggested by Johannes
- Use rule variables from the search results instead of search parameters, as using search parameters are a little fragile
- Suggested rule variables: transportmode/transportsubmode and authority. Should be safe showing message for AtB shuttle buses
db0d9f4
to
869d9a1
Compare
👆🏽 Force push because of rebase on |
@@ -102,7 +102,7 @@ function transportModeToEnum( | |||
transportMode: enumFromString(TransportMode, internal.transportMode), | |||
transportSubModes: internal.transportSubModes | |||
?.map((submode) => enumFromString(TransportSubmode, submode)) | |||
.filter(Boolean) as TransportSubmode[], | |||
.filter(isDefined), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a cast I noticed that could be replaced with something safer :)
Closes AtB-AS/kundevendt#19786
There are no Figma sketches for this, but here is a screenshot: