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

Pattern viewer configuration and styling #1336

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

amy-corson-ibigroup
Copy link
Contributor

Description:

  • Adds config item sortRoutePatternsByVehicles which makes sorting the route patterns by vehicle count in the dropdown optional.
  • Fixes some minor styling issues for the pattern viewer select dropdown, to prevent the headsigns from clipping.

PR Checklist:

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?
Before After
image image
image image

|

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.

We can make this much smaller!

Comment on lines 115 to 119
)

const headsigns = sortPatternsByVehicleCount
? unsortedHeadsigns.sort((a, b) => {
// sort by number of vehicles on that pattern
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can avoid adding the new variable and the ternary if you instead add an if(!sortPatternsByVehicleCount) return 0 at the top of the original sort method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't it more efficient to avoid iterating through the sort if we don't have to?

Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup Jan 9, 2025

Choose a reason for hiding this comment

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

If we always return 0 it's still O(n) and the code is much cleaner

lib/components/viewers/route-details.tsx Outdated Show resolved Hide resolved
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.

Wonderful!

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