Adjust layout slightly for small (/non-fullWidth) ALVIs#2006
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2006 +/- ##
==========================================
+ Coverage 43.19% 43.22% +0.02%
==========================================
Files 575 575
Lines 24747 24747
Branches 8216 7343 -873
==========================================
+ Hits 10689 10696 +7
- Misses 13382 14002 +620
+ Partials 676 49 -627 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
I'm okay with this in terms of it working, but I'm really not keen on having a bunch of exceptions at specific screen sizes owing to page layouts changing – if we ever change the sidebar layout, this'll all need to change too. I think this would be much improved with a container-based CSS approach, where the layout responds to the size of the list view object rather than the screen size. We do something similar for event cards, question metadata and a couple of other things. See example here. With this, I should think we can just say "if the object width is below a certain threshold, wrap the tags and auto-set full width", and we won't need e.g. the forceFullWidth map.
I'd be happy to merge this as is but create a card to rework list views into this container-based approach (the scope is much larger than just this adjustment PR), if you agree that it's a good way forwards. What do you think?
|
|
||
| fullWidth = fullWidth || below["sm"](deviceSize) || (isItem && !(typedProps.status || typedProps.audienceViews)); | ||
| const fullWidth = forceFullWidth || below["sm"](deviceSize) || (isItem && !(typedProps.status || typedProps.audienceViews)); | ||
| const wrapTitleTags = below["xs"](deviceSize) || (isDefined(forceFullWidth) && !forceFullWidth); |
There was a problem hiding this comment.
I don't love false and undefined representing different things for a boolean option, but we wouldn't need this at all with the container approach so I'm happy to leave it as is for now.
|
@jacbn I'll admit that this was a little hackier than I'd've liked. My original intention had just been changing the |
jacbn
left a comment
There was a problem hiding this comment.
Perfect, let's do that then – I'll make a card.
At explicitly small ALVI widths (when page is <=xs, or
fullWidthset to exactlyfalse), wrap title tags such as "DEPRECATED" and "NO-FILTER" to below the title so they take up less horizontal space.Also change
fullWidthrequirements on "My progress" from <=lg to {sm, lg, xl} to allow md pages to use a non-stacked layout (at full page-width), while also enabling it for xl pages to account for the new question status space requirements (at half page-width).Also requires isaacphysics/isaac-api#760