feat: add status filters on overview page #840
Conversation
|
Hi @arnaudvalle, thank u for this PR. |
| content: ''; | ||
| background-color: var(--item-bg); | ||
| border-radius: 50%; | ||
| width: 0.5rem; | ||
| height: 0.5rem; | ||
| display: inline-block; | ||
| display: flex; |
There was a problem hiding this comment.
You're right, this is not necessary anymore. I think I added it before changing the markup of the filters.
|
|
||
| export const OverviewPage = () => { | ||
| const { actions, queues } = useQueues(); | ||
| actions.pollQueues(); | ||
|
|
||
| const selectedStatus = useSelectedStatuses(); | ||
| const overviewPageStatus = selectedStatus['']; |
There was a problem hiding this comment.
selectedStatus['']- can u explain this statment?
There was a problem hiding this comment.
It was my understanding that this would be the queue value for the overview page, see in the useActiveQueueName hook?
But I guess this is probably not a good idea to rely on this since the overview page isn't really a queue?
| })}> | ||
| <NavLink | ||
| to={`/?status=${status}`} | ||
| className={s[toCamelCase(status)]} |
There was a problem hiding this comment.
There is status on the parent li, why do we need here?
There was a problem hiding this comment.
I believe that can go as well - that was before I moved the logic out to the <li> so it's redundant indeed 👍
|
|
||
| export const StatusLegend = () => { | ||
| const { t } = useTranslation(); | ||
| const { search } = useLocation(); |
So you mean if I only ever have queues with FAILED jobs (like in my screenshot) then I would only ever show the FAILED tab filter in the list and not the other ones? Something like: 
I'm not sure about how other people use this page tbh but if you say that's ok then it's fine by me 👍 |
No, i ment filter queues only in client side, "show me queues that have count on selected status that is greater than 0" |
I see - I believe that's how I got started on this but somehow got lost along the way... I'll fix the condition on the |
|
Instead of making a request with the "selected status", we can filter out all queues which has count of "selected status" > 0. |
|
Oh so you mean move the |
|
No, I mean that instead of filter queues with an API call, filter it only in client side. |
Ok sorry, I must be misunderstanding something here. I thought that's already what my change was doing in the OverviewPage? The Maybe it's my fault and my screenshot was misleading to star with (since I was only showing queues with failed jobs), but when I have a queue with both completed AND failed jobs for example, I can either see the queue when I select the completed is selected: failed is selected: no status selected: status with no job in any queues is selected: Thanks for your patience and feedbacks btw! |
|
Ha, OK... I'll merge it as soon as I will be near a keyboard (I'm on vacation right now) |
|
Oh, no rush, don't worry! I'll stop bothering you now, enjoy your time off 👍 |
|
I've refactor a bit the code... |
|
Great stuff, thanks for the cleanup 👍 |
Following the discussion on #742, I added some simple links to the
StatusLegendso it's now possible to filter queues based on a given status. This can be useful to direct link to a list of failed queues from some alerting system for example:I've tried to reuse as much as I could from existing components but I'm unsure whether you tend to duplicate similar things or not to future-proof them (for example I've reused css vars from
status-menuin the StatusLegend module css but maybe that's an anti-pattern for you?).Also, a current known limitation to this is that there is no
ALLlink (but we can click on the logo for example to go back to the initial state) as I wasn't sure on how best to handle this (I'm a first-time contributor to this project so I'm not comfortable with the architecture of it all just yet). Would adding a new status and associated translation for this make sense?