[web] Do not reserve space for section icons#549
Merged
Conversation
To allow having no indentation in pages mounting sections without icons. It's achieved by not using a fixed width but sizing the column with `min-content` [1] CSS keyword instead. [1] https://developer.mozilla.org/en-US/docs/Web/CSS/min-content
dgdavid
force-pushed
the
improve-sections-indentation
branch
from
0d07572 to
1a099b7
Compare
Pull Request Test Coverage Report for Build 4797421061
💛 - Coveralls |
joseivanlopez
approved these changes
Apr 25, 2023
Contributor
joseivanlopez
left a comment
joseivanlopez
left a comment
There was a problem hiding this comment.
If I am not wrong, we don't mix sections with and without icon yet. So I think the fix is a good compromise for now.
dgdavid
added a commit
that referenced
this pull request
Nov 28, 2023
A kind of rollback of #549 to avoid UI changes when a section without icon enters in a loading mode and renders the "section loading icon".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The initial sketch of Agama UI included icons close to the section titles, not only for aesthetic reasons but as a visual aid for people with no sight problems. They can help to quickly identify a section and also to create an indentation that helps to identify at first sight the section related content.
But, sadly, we do not have icons for all sections. What is more, we can have full pages without a section icon, which is the case of the Storage Proposal Page right now.
For these pages, the indentation looks not justified or a bit wrong, specially when adding elements that do not obey the section layout.
Solution
Since at this time we're using a CSS grid for laying out the section elements, using a not fixed value for the "icon" column looks like a good compromise fix.
The only caveats I'm able to see at this time are,
loadingprop in a section not using a representative icon (and, thus, reserving the space) will cause an ugly space distribution.But, as mentioned, this is a compromise fix, and we can see these caveats as a guideline of things we must avoid doing. What is more, for a section without icon the loading prop would be ignored and it must use a Skeleton instead. But that it's for another PR, if this got merged.
Testing