GAUD-8851 remove text scss#7161
Conversation
- Creates the input-text-styles.js file - Implements the style function generator - Mimics the text.scss styles based on the input-styles.js
…y input-text-styles.js
There was a problem hiding this comment.
I thought that would be easier to understand and implement style in BSI, therefore I created this new file with the generator function.
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
There was a problem hiding this comment.
What I did here was basically expose some base-style-generator-functions that I used in the new file input-text-styles.js
There was a problem hiding this comment.
I will revert these changes in the test file. The report results are really good and there is not significant difference
There was a problem hiding this comment.
this is a good practice that @dlockhart suggested me last time, and I found it very useful. Here we might see how different (if there is any) the component would look with these generated styles, compared to the sass ones
There was a problem hiding this comment.
I will revert these changes in this file. The report results are really good and there is not significant difference
| import { _isValidCssSelector } from '../../helpers/internal/css.js'; | ||
| import { getFocusVisibleStyles } from '../../helpers/focus.js'; | ||
|
|
||
| function _generateInputTextDisabledStyles(selector) { |
There was a problem hiding this comment.
This shouldn't be needed anymore. If you check the input-styles under the off flag. It has the same styles, meant to override focus-styles when item is disabled. However, the new generated styles adds :not(disabled) to those focus styles so they don't need to be overriden
There was a problem hiding this comment.
Thanks for the clarification. That answers my comment below as well.
| @@ -0,0 +1,54 @@ | |||
|
|
|||
| import { | |||
| _generateInputAriaInvalidBaseStyles, | |||
There was a problem hiding this comment.
Is the plan to use this functions separately on BSI? I think it would be cleaner to merge everything that is always used together under a single function so that consumers wouldn't need to import all of them separately.
There was a problem hiding this comment.
No, The plan is to only use the exported function _generateInputTextStyles from this file. I used only these ones to build it up easier and readable
| @@ -166,8 +166,15 @@ describe('d2l-input-text', () => { | |||
|
|
|||
| describe('sass', () => { | |||
There was a problem hiding this comment.
I mentioned this on the other PR but input-text was using the new generated styles because the flag defaults to true so the base tests are already tracking the changes, these could be removed along the sass styles
There was a problem hiding this comment.
Oh you're correct. Thank you for the heads up!
…hey are already handled by the Input base styles by the :not(:disabled)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
Jira
GAUD-8851
I strongly suggest to review this commit by commit to understand the final result in case it doesn't make sense right away