Closed (fixed)
Project:
Project Browser
Version:
1.0.x-dev
Component:
User experience
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
18 May 2022 at 17:18 UTC
Updated:
21 Jun 2022 at 15:34 UTC
Jump to comment: Most recent, Most recent file


Comments
Comment #2
hooroomooComment #5
bnjmnmWith the MR: On narrow viewports, the categories collapse into a details element. There's currently no designs for mobile but this isn't pretending to be the official design. This is to address the several-scrolls of categories for mobile users using as much native html as possible.

There's a bit of opinoinated-ness in the MR, though. Some views classes were removed from elements because they largely (or only) added styling that was then un-styled in the Svelte component. If a class is adding styles we have to revert, it's probably better to remove the class. There will probably be more culling of views classes in future issues to simplfy component styling.
Comment #6
bnjmnmComment #7
chrisfromredfinI believe we should take the opportunity to clean up unused classes so I've updated the title. :)
I think this is the right approach; if there's somethign in Claro we can use, use it. If we can't, make it as natively-supported as possible.
My only feedback is that I believe the categories are "secondary" to the main keyword search in the UI hierarchy, so I would be happier if we could put the Categories details element after the keyword search. With that said, now that we've moved the markup around in terms of pulling the categories up, I assume this is a lot harder. Does @hooroomoo have any ideas about how to refactor that with grid or something so we can re-flow without JS?
Comment #8
fjgarlin commentedComment #9
chrisfromredfinTaking back what I said after talking to @bnjmnm in Slack today - switching source order with Grid is not accessible so I say for now let's just keep categories above.
Comment #10
fjgarlin commentedThere is still one line that contains old changes. I made a comment saying what should be the right line.
Comment #11
chrisfromredfinI made the suggested fix, but it now needs a rebase that I'm not in tune with.
Comment #12
bnjmnmComment #13
bnjmnmComment #14
hooroomooComment #15
hooroomooManually tested and works great. Found one nit in code but otherwise it looks good to me. Can probably self-RTBC after.
Comment #17
narendrarHi @bnjmnm, is making categories as Fieldset make them more accessible, as done in https://www.drupal.org/project/project_browser/issues/3282700? If Yes, we can copy changes done in that ticket and close that ticket.
Comment #18
bnjmnmRe #17
Using fieldset to group the checkboxes is a good accessibility improvement so it should happen. It would not, however, solve the issue being addressed here, which is too much screen area devoted to category checkboxes on smaller devices. These changes will not conflict with one another, and solve different problems, so lets do both and keep them in their respective issues.
Comment #19
hooroomooComment #21
chrisfromredfinBig improvement, thanks!