Skip to content
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

Sync itinerary sort with depart arrive #1266

Merged
merged 12 commits into from
Sep 26, 2024

Conversation

daniel-heppner-ibigroup
Copy link
Contributor

Description:
This syncs the depart arrive setting from the date time selector with the sort option for the itineraries.
It also adds an associated configuration option.

},
[onSortChange]
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this useCallback is probably hurting performance more than anything. onSortChange won't be recreated on renders anyway since it's being passed in

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love!

Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big love, I'd just like to see this disabled by default

lib/components/form/date-time-modal.tsx Outdated Show resolved Hide resolved
},
[onSortChange]
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love!

@daniel-heppner-ibigroup
Copy link
Contributor Author

Thank you! That map was a really good suggestion!

Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the typescript!

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work, see a few suggested code tweaks.

example-config.yml Show resolved Hide resolved
lib/components/form/date-time-modal.tsx Outdated Show resolved Hide resolved
lib/components/form/date-time-modal.tsx Outdated Show resolved Hide resolved

import { AppConfig } from '../../util/config-types'
import { AppReduxState, FilterType, SortType } from '../../util/state-types'
import { setQueryParam } from '../../actions/form'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write import * as formActions from ... and adjust in mapDispatchToProps accordingly, so that the name of the function is not ambiguous inside the component (2 instances).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the other instance?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the other instance?

Actions imported from narrative.

lib/components/form/date-time-modal.tsx Outdated Show resolved Hide resolved
lib/components/form/date-time-modal.tsx Outdated Show resolved Hide resolved
lib/components/form/date-time-modal.tsx Show resolved Hide resolved
@daniel-heppner-ibigroup
Copy link
Contributor Author

hadn't realized you had approved, binh. merging now.

@daniel-heppner-ibigroup daniel-heppner-ibigroup merged commit ca8995a into dev Sep 26, 2024
9 checks passed
@daniel-heppner-ibigroup daniel-heppner-ibigroup deleted the sync-sort-departarrive branch September 26, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants