chore: Fix sass deprecation warnings #2638
Merged
Conversation
This file contains 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
just-boris
force-pushed
the
fix-sass-warning
branch
from
d8d94d2
to
1c39325
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2638 +/- ##
=======================================
Coverage 96.00% 96.00%
=======================================
Files 749 749
Lines 20896 20896
Branches 7070 7119 +49
=======================================
Hits 20062 20062
Misses 780 780
Partials 54 54 ☔ View full report in Codecov by Sentry. |
just-boris
force-pushed
the
fix-sass-warning
branch
from
1c39325
to
17a8bbd
Compare
just-boris
force-pushed
the
fix-sass-warning
branch
from
17a8bbd
to
d4a0b10
Compare
just-boris
force-pushed
the
fix-sass-warning
branch
from
d4a0b10
to
c0d9785
Compare
just-boris
force-pushed
the
fix-sass-warning
branch
from
c0d9785
to
02fc216
Compare
just-boris
force-pushed
the
fix-sass-warning
branch
from
02fc216
to
1535271
Compare
just-boris
commented
Sep 17, 2024
| @@ -78,6 +78,7 @@ function stylesTask(theme) { | |||
| designTokensFileName: theme.designTokensOutput, | |||
| descriptions, | |||
| jsonSchema: true, | |||
| failOnDeprecations: true, | |||
There was a problem hiding this comment.
Currently there are zero deprecation warnings. This line is to make sure it the build fails if anybody attempts using deprecated setup
just-boris
commented
Sep 17, 2024
Comment on lines
155
to
161
| &-disable-paddings { | ||
| margin-block: 0; | ||
| grid-column: 1 / -1; | ||
| @include desktop-only { | ||
| grid-column: 2 / span 3; | ||
| } | ||
| margin-block: 0; | ||
| } |
There was a problem hiding this comment.
This snippet demonstrates the deprecated pattern very clear. Top level styles should always go first, before any nested rules.
just-boris
commented
Sep 17, 2024
Comment on lines
-66
to
-70
| /* | ||
| Warning! If these design tokens for width change it will adversely impact | ||
| the calculation used to determine the Split Panel maximum width in the | ||
| handleSplitPanelMaxWidth function in the context. | ||
| */ |
There was a problem hiding this comment.
This comment is copy-paste from VR app layout to the toolbar version, but it is not relevant anymore, because the implementation changed completely
just-boris
commented
Sep 17, 2024
Comment on lines
-203
to
-204
|
|
||
| @include styles.styles-reset; |
There was a problem hiding this comment.
I do not know what to say here. Hiding the reset line here is a readability issue, great that the new deprecation forces us to fix this
dpitcock
previously approved these changes
Sep 18, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
just-boris
force-pushed
the
fix-sass-warning
branch
from
1535271
to
845400c
Compare
dpitcock
approved these changes
Sep 19, 2024
johannes-weber
pushed a commit
that referenced
this pull request
Oct 4, 2024
Before this change, the resizer button was relatively positioned when focused. The relative position was coming from the `focus-highlight` mixin. This regression was introduced when fixing sass deprecation warnings (#2638). Wrapping the absolute position in `& {}` creates a separate rule which takes precedence because of the rule declaration order.
johannes-weber
pushed a commit
that referenced
this pull request
Oct 4, 2024
Before this change, the resizer button was relatively positioned when focused. The relative position was coming from the `focus-highlight` mixin. This regression was introduced when fixing sass deprecation warnings (#2638). Wrapping the absolute position in `& {}` creates a separate rule which takes precedence because of the rule declaration order.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Resolves #2571
Related links, issue #, if available: n/a
How has this been tested?
Screenshot tests in my pipeline
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.