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

Incorporate "Priority" levels in sorting #6

Closed
benmelz opened this issue Aug 18, 2023 · 7 comments · Fixed by #23
Closed

Incorporate "Priority" levels in sorting #6

benmelz opened this issue Aug 18, 2023 · 7 comments · Fixed by #23
Assignees
Labels
enhancement New feature or request

Comments

@benmelz
Copy link
Member

benmelz commented Aug 18, 2023

Originally posted by @sherson in #4 (comment)

...

However, now that I think about it, we ought to sort by Priority too. How about:

  • Group routes together (sort by route within each detour)
  • Sort detours by Priority, then ascending by route (where general messages [ones that aren't associated with any routes] are after route-specific messages at the same priority level, and we use the lowest route number when there are multiple routes associated with a single detour), then descending by the date portion of FromDate, then descending by the time portion of FromTime (so newest detour shows first if route and priority are the same)

Priority values appear to be:

  1. Emergency
  2. High
  3. Medium
  4. Low

No matter what we do, the nature of grouping by route means we may occasionally end up with detours for a specific route not being next to each other in the list (and sorting by priority will affect that as well).

I have a vague recollection of FromTime maybe not being used (or not being used in a way that make sense), and what there's now is weird (explained in this comment):

<Message>
Due to staffing levels, "S" Trips will not operate between 05/27/2023 and 09/03/2023.
</Message>
...
<FromDate>2023-05-28T00:00:00</FromDate>
<FromTime>2023-05-30T00:00:00</FromTime>

But I can't think of harm being caused by sorting by FromDate and then FromTime.

Am open to suggestions.

@benmelz benmelz added the enhancement New feature or request label Aug 18, 2023
@benmelz
Copy link
Member Author

benmelz commented Aug 18, 2023

I have a vague recollection of FromTime maybe not being used (or not being used in a way that make sense), and what there's now is weird:

AFAIK the start/date time validity depends on how someone defines the public message in avail and can definitely not make sense (since someone might just pick a large arbitrary date range if it's unknown at the time and then go in and deactivate it when it's done).

As such I'm thinking it's not worth it to add yet another level of sorting for it

@sherson
Copy link
Member

sherson commented Aug 18, 2023

Had a chance to look back through old Avail issues. From https://github.com/umts/pvta-avail/issues/1932:

the time portion of FromDate and ToDate is always midnight and should be ignored, and the date portion of FromTime and ToTime is always "today" and should be ignored.

I think it's still worth sorting in descending order by the time portion of FromTime so if we create multiple detours during a snow storm, it'll show the newest one at the top (within priority and route sort order), but only if it's not too much extra work to add yet another sorting level.

@benmelz
Copy link
Member Author

benmelz commented Aug 18, 2023

Ok so to sum up the plan for this, the sorting of public messages will consider, in order:

  • Priority
  • SortOrder of the highest ranked route in a route group (general messages at the bottom)
  • Date portion only from FromDate (comes in as ISO with worthless time)
  • Time portion only from FromTime (comes in as ISO with worthless date)

I don't think we need to preserve lexical ordering for messages as a final fallback -FromDate + FromTime will almost never be non-unique and at that point, it probably doesnt matter

@frothedoatmilk
Copy link
Contributor

I spoke to @benmelz about this briefly but we wanted to get your opinion @sherson, for each priority level we could add some symbol/color to each message to indicate it's priority level (red for emergency, orange for high, etc). Especially since this would show the order of which the messages are appearing in, rather than it being seemingly random (especially since the sort order doesn't make apparent sense from the front end)

@sherson
Copy link
Member

sherson commented Aug 21, 2023

Seems reasonable, though if that ends up increasing the height of each item, it might not be a bad idea to make it optional (via URL parameter) since we're displaying it in a small iframe.

For reference, if you download the Avail myStop app, you can see how they render it.

Not sure if Transit app distinguishes between priority levels... at a quick glance I don't see a difference between low and medium for the two public messages currently posted (both have yellow warning triangles).

@frothedoatmilk
Copy link
Contributor

The Transit app somehow classifies each posting, right now there's a G1 delay that's labelled as an accident, but the ongoing stuff with B7/B17 has no classification? Could be completely unrelated though.

In my head, we would add a bar of color to the left of the whole message, something like this:
Screenshot 2023-08-22 at 10 40 48 AM

Obviously we would debate the colors a bit more (yellow on white => ???). Ben's idea could be appended to the right as an additional column (each message being rendered right now as a table row), we would probably want to pack in FontAwesome or just grab a couple cute icons. Not sure which is more efficient/better for performance?

@benmelz
Copy link
Member Author

benmelz commented Aug 22, 2023

I might be a fan of just colored bars as you have here. As I did with this issue, I think I'm gonna break it out to keep to another ticket though, so we can move this convo there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants