refactor(slider): use relative units for si-slider#2022
refactor(slider): use relative units for si-slider#2022
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the slider component's layout from flexbox to CSS grid and converts several dimension variables from pixels to relative rem units. Feedback suggests ensuring all dimension variables use consistent rem units for better adherence to the project's objectives, removing unused variables, and utilizing the $button-width variable within the grid template to improve maintainability and layout consistency.
I am having trouble creating individual review comments. Click here to see my feedback.
projects/element-ng/slider/si-slider.component.scss (3-8)
To fully adhere to the PR's objective of using relative units, all dimension variables should be converted to rem. Additionally, $track-height and $thumb-size appear to be unused in this stylesheet; consider removing them if they are not needed for the component's styling.
$button-width: 2rem;
$button-height: 2rem;
$button-gap: 0.25rem;
$track-height: 0.25rem;
$thumb-size: 1.5rem;
References
- When performing calculations in SCSS involving variables, ensure that the units of the variables are consistent (e.g., all pixels) if the calculation is to be resolved at compile time by Sass. If units are mixed (e.g., rem and px), use the CSS calc() function to ensure the browser handles unit resolution correctly at runtime.
projects/element-ng/slider/si-slider.component.scss (17)
The $button-width variable should be utilized in the grid-template for the button columns. This ensures the layout is driven by the defined variables rather than relying on auto sizing, improving consistency and maintainability.
grid-template: auto $button-height / $button-width 1fr $button-width;
|
Because @github is broken again I cannot comment in the code... This: button dimensions should be aligned with our circle buttons I think: |
|
i.e. @github messing up..sometime with error, sometimes silently dropping the comment. Nice.
|
c1bfa17 to
72a214f
Compare
There was a problem hiding this comment.
I guess these need some rem/calc magic too?
37aa972 to
dc00560
Compare

See https://code.siemens.com/simpl/simpl-element/-/work_items/4
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: