The "sort by" dropdown has a fixed width of 152px. This looks nice and helps conserve space in the container it resides in, but it doesn't take into account that a longer option might be selected. In English, only the "Date Created (most recent)" option is cut off, but this could impact more options depending on the language used.
It's not possible to fully see certain options. This could be especially difficult if there are options that begin the same but end differently (such as ending with asc/desc)
This should probably go in even if it "breaks" the design, as the design needs to be able to elegantly handle a certain amount of variation in option length. Having this in HEAD will make it easier to experience what's happening and come up with a good design solution.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | after.png | 11.75 KB | Munavijayalakshmi |
| #13 | before.png | 11.3 KB | Munavijayalakshmi |
| #9 | Screenshot 2022-07-08 at 08.46.54.png | 19.48 KB | fjgarlin |
| #9 | Screenshot 2022-07-08 at 08.46.24.png | 25.14 KB | fjgarlin |
| #8 | after(1).png | 15.73 KB | omkar-pd |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
vighneshh commentedComment #5
vighneshh commentedMade some css changes and added before and after screenshots . Please Review
Comment #6
fjgarlin commentedThe tests are failing. Whenever you do changes, you need to run "yarn build" so it includes the compiled assets.
Also, the "after.png" still shows how the right parenthesis is nearly cut-off, so maybe a bit of extra spacing is needed on the right side.
Comment #8
omkar-pd commentedremoved fixed height and added build files.
Comment #9
fjgarlin commentedMarking this as RTBC based on
It does look good in the longer option:
And a bit weird in the not-so-long options:
Perhaps we should also shorten "Date Created (Most Recent)" to "Recently created", but I'm not sure if that's in the scope of this issue. If we do it as part of this issue, it's a one-line change.
Comment #10
tim.plunkettThis needs a rebase anyway due to recent commits, let's also make the change suggested in #9 (please change the reference to it in tests as well).
Comment #11
omkar-pd commentedMade all the changes.
Comment #12
Munavijayalakshmi commentedComment #13
Munavijayalakshmi commentedComment #14
Munavijayalakshmi commentedComment #16
tim.plunkettThanks for making the change!