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

Location Groups July 2024 #1017

Open
wants to merge 25 commits into
base: dev-flex-2024
Choose a base branch
from

Conversation

miles-grant-ibigroup
Copy link
Contributor

@miles-grant-ibigroup miles-grant-ibigroup commented Jul 11, 2024

Location Group support following latest flex spec. Works with data tools-server dev-flex branch

@miles-grant-ibigroup miles-grant-ibigroup self-assigned this Jul 11, 2024
@miles-grant-ibigroup miles-grant-ibigroup changed the base branch from dev to dev-flex-2024 July 11, 2024 18:43
@miles-grant-ibigroup miles-grant-ibigroup added WIP BLOCKED Blocked (waiting on another PR to be merged) Flex labels Jul 11, 2024
@clinephi
Copy link

Excited for this update!

@ibi-group ibi-group locked as too heated and limited conversation to collaborators Sep 12, 2024
@miles-grant-ibigroup miles-grant-ibigroup marked this pull request as ready for review September 17, 2024 14:36
@miles-grant-ibigroup miles-grant-ibigroup removed the BLOCKED Blocked (waiting on another PR to be merged) label Dec 16, 2024
Copy link
Contributor

@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.

First round of comments.

lib/editor/actions/location.js Outdated Show resolved Hide resolved
lib/editor/actions/location.js Show resolved Hide resolved
lib/editor/actions/map/stopStrategies.js Show resolved Hide resolved
lib/editor/components/map/PatternStopsLayer.js Outdated Show resolved Hide resolved
lib/editor/actions/map/stopStrategies.js Outdated Show resolved Hide resolved
lib/editor/components/pattern/EditShapePanel.js Outdated Show resolved Hide resolved
lib/editor/components/pattern/PatternStopCard.js Outdated Show resolved Hide resolved
lib/editor/components/pattern/PatternStopContainer.js Outdated Show resolved Hide resolved
@miles-grant-ibigroup miles-grant-ibigroup self-assigned this Feb 3, 2025
Copy link
Contributor

@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.

Next round of comments...

lib/editor/components/timetable/TimetableEditor.js Outdated Show resolved Hide resolved
lib/editor/reducers/data.js Show resolved Hide resolved
lib/editor/selectors/timetable.js Outdated Show resolved Hide resolved
lib/editor/util/map.js Show resolved Hide resolved
lib/editor/util/map.js Show resolved Hide resolved
lib/editor/util/map.js Outdated Show resolved Hide resolved
lib/editor/util/map.js Show resolved Hide resolved
Copy link
Contributor

@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.

Nice cleaning up the interim flex stuff and improving the types. I am going to give the benefit of doubt regarding the flow stuff. There are few nits but nothing fundamentally blocking.

lib/editor/actions/map/stopStrategies.js Show resolved Hide resolved
lib/editor/components/map/PatternStopsLayer.js Outdated Show resolved Hide resolved
lib/editor/components/map/PatternStopsLayer.js Outdated Show resolved Hide resolved
lib/editor/components/pattern/PatternStopCard.js Outdated Show resolved Hide resolved
@@ -863,6 +824,13 @@ export type GtfsLocation = {|
zone_id?: string
|}

// export type GtfsLocationGroup = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll need it to show location groups on the map.

@binh-dam-ibigroup binh-dam-ibigroup removed their assignment Feb 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants