Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Deprecate the $spacing- variables #10686

Merged
merged 2 commits into from
Apr 21, 2023
Merged

Deprecate the $spacing- variables #10686

merged 2 commits into from
Apr 21, 2023

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Apr 21, 2023

... and write some comments about the $font- variables.

I think this is an accurate representation of what we discussed in our meeting yesterday, but would appreciate feedback.

If we're agreed on this I will follow up with an update to the code_style doc in element-web.


This change is marked as an internal change (Task), so will not be included in the changelog.

@richvdh richvdh requested a review from a team as a code owner April 21, 2023 10:00
@richvdh richvdh added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Apr 21, 2023
@richvdh richvdh requested a review from germain-gg April 21, 2023 10:00
@luixxiul
Copy link
Contributor

Will this deprecate this PR? #10552

Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

Comment on lines +22 to +25
* In future, we plan to introduce variables named according to their purpose rather than their size. Additionally,
* we want switch to custom CSS properties (https://github.com/vector-im/element-web/issues/21656), so we might have
* `--spacing-standard` or something. For now, you might as well use hardcoded px values for lengths (except for font
* sizes, for which see the `$font-<N>px` variables).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this will be done as part of what https://github.com/vector-im/compound-design-tokens outputs.
I've commented on the linked issue to explain a rough path to a better future

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@artcodespace artcodespace removed their request for review April 21, 2023 11:41
@richvdh richvdh added this pull request to the merge queue Apr 21, 2023
@richvdh
Copy link
Member Author

richvdh commented Apr 21, 2023

Will this deprecate this PR? #10552

Given element-hq/element-web#21656 (comment), I think it probably does, yes.

richvdh added a commit to element-hq/element-web that referenced this pull request Apr 21, 2023
Per matrix-org/matrix-react-sdk#10686, these don't make
much sense at the moment.
richvdh added a commit to element-hq/element-web that referenced this pull request Apr 21, 2023
Per matrix-org/matrix-react-sdk#10686, these don't make
much sense at the moment.
Merged via the queue into develop with commit 5fc402c Apr 21, 2023
@richvdh richvdh deleted the rav/deprecate_spacing_vars branch April 21, 2023 14:12
@luixxiul luixxiul mentioned this pull request Jun 1, 2023
3 tasks
germain-gg pushed a commit that referenced this pull request Jun 6, 2023
The variables on _spacing.pcss have been deprecated by #10686
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants