Conversation
dgdavid
force-pushed
the
ligthen-web-ui-internals
branch
from
1ebe5b9 to
e15a9d0
Compare
Because it does not look alive anymore.
By using CSS Grid Layout[1], CSS Flexible Box Layout (aka Flexbox), and starting to set the foundations to follow the CubeCSS[3] methodology. [1] https://www.w3.org/TR/css-grid-1/ [2] https://www.w3.org/TR/css-flexbox-1 [3] https://cube.fyi/
I.e., Flex, Split, Stack, and Bullseye. The latest also include our Center component. They are going to be replaced by custom CSS utilities, which hopefully will help to reduce the components complexity and the amount of DOM nodes.
Because latest Firefox versions does not support the rationale (:has) pseudo-selector yet. * https://www.w3.org/TR/selectors-4/#relational * https://developer.mozilla.org/en-US/docs/Web/CSS/:has#browser_compatibility
Using a native unordered list instead of PatternFly/Card, which was not designed for building lists[1]. [1] https://www.patternfly.org/v4/components/card/design-guidelines
Since we're using an specific rule in webpack.config.js file for fonts, we can't rely on @use Sass at-rule for importing the fonts.scss file. Instead, we have to import it in the index.js file directly, for making webpack aware of it. What is more, most probably the fonts.scss file could be renamed to fonts.css since it does not contain nothing to process with Sass. Related to #385
Removing dead Sass variables too.
dgdavid
force-pushed
the
ligthen-web-ui-internals
branch
from
f128944 to
35f4a11
Compare
imobachgs
reviewed
Jan 11, 2023
Contributor
imobachgs
left a comment
imobachgs
left a comment
There was a problem hiding this comment.
Great work! Just a few typos. Otherwise, it looks good to me.
Namely, * Fixed some typos * Removed leftover CSS class * Remove dead CSS comments * Improved some onClick bindings
dgdavid
force-pushed
the
ligthen-web-ui-internals
branch
from
959650f to
fcecd1c
Compare
imobachgs
approved these changes
Jan 11, 2023
This was referenced Jan 11, 2023
This was referenced Jan 23, 2023
Closed
Merged
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.
Over Christmas, I started to rework the web UI internals to simplify how we build it.
Among other topics, I had concerns about the amount of DOM nodes used for laying out the UI[1]. Of course, browsers are so performant these days, and the D-Installer's web UI is far from troublesome in that regard. In the end, it is rather simple compared to the UI of medium/large web applications. However, I think it's a worthwhile idea to stop using layout components (PF4/Split, PF4/Stack, and friends) and rely more on plain CSS.
I also took the opportunity to start applying the CUBE CSS methodology after watching the Be the browser’s mentor, not its micromanager talk from its author, Andy Bell.
It is true that PatternFly, the component library we're using, uses BEM instead. Still, CUBE looks simpler to me (yet somehow BEM compatible). So why not give it a try?
As a whole, there is still work to be done. However, this PR can be seen as a first step[2] for rebuilding the UI internals by introducing CUBE CSS, leaving behind the layout components and using PatternFly strictly as a component library.
Screenshots
Some metrics
Click to show/hide some not so relevant numbers
.getElementsByTagName("*")
.length
(thanks to domVisualizer)
Next steps
In no particular order, below are the next UI changes I'd like to make
To fix the misuse of buttons/links
https://ashleemboyer.com/blog/should-i-use-a-button-or-a-link
Use the section headers as links to the corresponding page
After playing with it, I found that using the underlined link[3] is the best, more intuitive way for the user to know there is a page. Even more considering that all sections in the summary will potentially have their Page counterpart.
Better CUBE CSS implementation (along with better CSS variables naming and all related stuff for having a themeable UI)
Start using a fluid type scale
https://www.fluid-type-scale.com
Introduce Design Tokens to simplify the process of creating themes or just applying changes (not sure, need to read more about it)
https://tr.designtokens.org/format/
https://tokencss.com/
https://equinusocio.dev/blog/design-tokens-misconceptions-and-theme-tokens/
...
[1] Premature optimization, I know. But I felt that we are/were in a phase in which we can explore and address these topics more easily.
[2] Or even an attempt, since it might not be the best/final solution. After all, I'm (re)learning by doing 😉
[3] it can be a highlighted link instead, but only if all section headers are links…. Anyway, something to discuss later in a different PR.