fix: added self-contained styles for spinner component #130
Conversation
|
Deploy preview for fundamental-styles ready! Built with commit 6d3b361 |
JYSW380
changed the title
components/spinner.scss
Outdated
| @@ -0,0 +1,4 @@ | |||
| $fd-fonts-path: "../scss/fonts/"; | |||
There was a problem hiding this comment.
I don't think this component needs fonts to work
There was a problem hiding this comment.
no, we dont need fonts, thanks for point out.
There was a problem hiding this comment.
@jbadan is correct. We should NOT be using aria-* attributes as CSS selectors. I know it's not technically part of making this self-contained styles, but this is a good time to clean some of that up.
Also, there is an unclassed div element selector in the scss/components/spinner.scss file. Although this div element selector is a child of .fd-spinner, this should be changed to use a class selector to avoid anything bleeding in or out.
greg-a-smith
changed the title
|
|
||
| {% set example %} | ||
| <div class="fd-panel" aria-busy="true"> | ||
| <div class="fd-panel is-busy"> |
There was a problem hiding this comment.
The "hidden" example should be redone as it no longer is triggered by aria-hidden. The text should be changed and the example itself should implement the is-hidden class.
…to fix/129-self-contained-styles-for-spinner-component
…to fix/129-self-contained-styles-for-spinner-component
…ttps://github.com/SAP/fundamental-styles into fix/129-self-contained-styles-for-spinner-component
There was a problem hiding this comment.
Sorry, I missed @jbadan's review comment. Added the missing variable to theme-ugly.css. Looks good though. 🚢
| --fd-color-background-3: rgb(47%, 36%, 91%); | ||
| --fd-color-background-4: rgb(51%, 6%, 2%); | ||
| --fd-color-alert: rgb(84%, 13%, 91%); | ||
| --fd-color-background-5: rgb(13%, 71%, 90%); |
There was a problem hiding this comment.
Really just reorganized/reordered these. The only addition was --fd-color-background-1.
|
this component ( this should be removed from the styles lib and we should work on the new ones instead. @greg-a-smith @jbadan @droshev thought? |
|
@saad-mo If this component is deprecated, I agree we should begin work on its replacements. That said, until those replacements exist, this needs to stay so we should push this PR through and create a GitHub issue to have it removed once the replacement components are available. |
Closes #127
This PR contains the self-contained styling for the spinner component.
This PR is part of the larger task of making all components self-contained. #136