Skip to content

Conversation

bencodeorg
Copy link
Contributor

A proposed organizational improvement to managing the modal in the side panel -- namely, using a single bit of state to choose which modal to show, rather than a series of booleans.

@coveralls
Copy link

coveralls commented Mar 26, 2025

Coverage Status

coverage: 16.097% (+0.6%) from 15.519%
when pulling 7cb61a7 on bencodeorg:ben/manage-modal
into c13b9b7 on OneBusAway:main.

Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

Definitely looks like a major improvement. I'm seeing one blocker preventing merging: trying to plan a trip doesn't work—the modal doesn't pop up.

@bencodeorg
Copy link
Contributor Author

Definitely looks like a major improvement. I'm seeing one blocker preventing merging: trying to plan a trip doesn't work—the modal doesn't pop up.

No big deal, right? 😅 Sorry, was planning on taking another pass through this before formally requesting review (does it auto-request when you open a PR?) Anyway, is there a public OTP endpoint or guidance on how to set it up locally (I could probably sort this without it, but curious regardless). Thanks!

@aaronbrethorst
Copy link
Member

@bencodeorg I slacked you the necessary info

@bencodeorg
Copy link
Contributor Author

Alrighty @aaronbrethorst, should be sorted now (was a typo, switched to use a constant to manage the modal states rather than individual strings to hopefully avoid in the future).

Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

looks good! 🔥

@aaronbrethorst aaronbrethorst merged commit d4b0cd3 into OneBusAway:main Mar 28, 2025
4 checks passed
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