Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: provide legacy CSS custom props for backwards compatibility #8355

Merged
merged 2 commits into from
Dec 7, 2023

Conversation

alisonailea
Copy link
Contributor

Related Issue: #8354

Summary

Include a map of new design tokens to old CSS Custom Props set previously by calcite-colors to provide legacy CSS custom props for backwards compatibility.

@alisonailea alisonailea added the p - high Issue should be addressed in the current milestone, impacts component or core functionality label Dec 6, 2023
@alisonailea alisonailea added this to the 2023 December Priorities milestone Dec 6, 2023
@alisonailea alisonailea self-assigned this Dec 6, 2023
@alisonailea alisonailea requested a review from a team as a code owner December 6, 2023 21:11
@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Dec 6, 2023
@@ -8,6 +8,9 @@
// Add Calcite Design Tokens as CSS Custom Props
@import url("~@esri/calcite-design-tokens/dist/css/index.css");

// Include legacy tokens for backwards compatibility
@import "./legacy";
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can make this a separate file they could add to their app instead of including it for everyone? Since it does add quite a bit of CSS. What are your thoughts there?

Copy link
Member

Choose a reason for hiding this comment

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

If we automatically include the old vars, it will be hard to see where vars still need to be converted within their apps.

Copy link
Member

Choose a reason for hiding this comment

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

We're including them in the global styles to allow us to deprecate and give enough time for teams to migrate. We can remove from the global styles, and possibly leave it up to users to add, at a later point.

Since it does add quite a bit of CSS

It adds some bits, but IMO, it's worth it to improve the migration process.

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Awesome! 🚀

@driskull
Copy link
Member

driskull commented Dec 6, 2023

@jcfranco would this be more useful as a separate global CSS file to provide backwards compatibility? That way it can optionally be added and removed for testing conversion to the new var name.

@benelan
Copy link
Member

benelan commented Dec 6, 2023

would this be more useful as a separate global CSS file to provide backwards compatibility? That way it can optionally be added and removed for testing conversion to the new var name.

Lol yeah I already tried that @driskull - https://github.com/benelan/calcite-codemod#using-the-css

I agree with Franco that adding it back is the more accommodating way of doing it though. Deprecating before removing so people have more time to transition makes sense to me.

@driskull
Copy link
Member

driskull commented Dec 6, 2023

IMO it would be better as an external file so a user could test their conversion. Otherwise, if they are unknowingly relying the the legacy vars we may brake their apps later.

@jcfranco
Copy link
Member

jcfranco commented Dec 6, 2023

@driskull If I'm following, this is where the deprecation messaging will come into play. cc @geospatialem @DitwanP

@benelan
Copy link
Member

benelan commented Dec 6, 2023

the deprecation messaging will come into play. cc @geospatialem @DitwanP

We should remove its breaking change from the CC changelog in addition to deprecating the legacy CSS variables in our documentation.

## [2.0.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@1.11.0...@esri/calcite-components@2.0.0) (2023-12-02)
### ⚠ BREAKING CHANGES
- __tokens:__ Changes to `@esri/calcite-design-tokens`, including the names of CSS variables used to customize component styling. See the [breaking changes section](https://github.com/Esri/calcite-design-system/blob/main/packages/calcite-design-tokens/CHANGELOG.md#200-2023-12-02) of the `@esri/calcite-design-tokens` changelog. ([#8311](https://github.com/Esri/calcite-design-system/issues/8311)) ([8d7cf3f](https://github.com/Esri/calcite-design-system/commit/8d7cf3f9bca3e908c1b0383209b348640c623084)), ([#8299](https://github.com/Esri/calcite-design-system/issues/8299)) ([4050a91](https://github.com/Esri/calcite-design-system/commit/4050a913d37fca76b79dfe97956a9ce2beef948c)), ([#8215](https://github.com/Esri/calcite-design-system/issues/8215)) ([335d010](https://github.com/Esri/calcite-design-system/commit/335d0106ef0f9d0ce71bda8d2c826bccfedc4995))

@driskull
Copy link
Member

driskull commented Dec 6, 2023

Yeah I guess this makes sense. Its just odd we're unbreaking a breaking change.

@jcfranco
Copy link
Member

jcfranco commented Dec 6, 2023

It's more of a softening vs unbreaking. 🤪🍦

@alisonailea
Copy link
Contributor Author

We should remove its breaking change from the CC changelog in addition to deprecating the legacy CSS variables in our documentation.

I'd like to leave it as is. I don't want to provide legacy support long term. I made a PR to update some other docs around this though.

#8357

@jcfranco
Copy link
Member

jcfranco commented Dec 6, 2023

Got some clarification from @alisonailea on ☝️. For calcite-components, we need to revisit the changelog to cover deprecation of legacy CSS props and also link to the design token's changelog. The calcite-design-tokens changelog can stay the same in terms of breaking changes. Ali, feel free to correct me.

@@ -0,0 +1,114 @@
// Map of Calcite Design Tokens to old CalciteComponent Custom Props (from calcite-colors)
:root {
Copy link
Member

Choose a reason for hiding this comment

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

Max left a comment earlier this morning regarding two additional props not mentioned below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added!

@alisonailea alisonailea added the skip visual snapshots Pull requests that do not need visual regression testing. label Dec 7, 2023
@alisonailea alisonailea merged commit b0f063e into main Dec 7, 2023
14 checks passed
@alisonailea alisonailea deleted the astump/8354-legacycss branch December 7, 2023 18:50
benelan added a commit that referenced this pull request Dec 7, 2023
…ts-node-to-tsx

* origin/main:
  chore: release next
  ci(next): install deps before running npm scripts (#8370)
  feat: provide legacy CSS custom props for backwards compatibility (#8355)
  fix(input-date-picker): no longer emits redundant change event (#8341)
  fix(shell-panel): adds border at the start when slotted in `panel-end` (#8314)
  build(deps): update dependency sortablejs to v1.15.1 (#8364)
benelan added a commit that referenced this pull request Dec 8, 2023
…ssage

* origin/main:
  build(node): bump node version to v20 (latest LTS) (#8381)
  build: swap ts-node for tsx (#8196)
  chore: release next
  build(deps): update dependency @types/prettier to v2.7.3 (#8375)
  build(deps): update dependency ng-packagr to v17.0.2 (#8362)
  test(tab-nav): stabilize test (#8356)
  fix(list-item): drag grid cell should be accessible via arrow keys. (#8353)
  chore: release next
  feat(list): Add `calciteListDragStart` and `calciteListDragEnd` events for when a drag starts and ends. (#8361)
  build(deps): update dependency tailwindcss to v3.3.6 (#8376)
  ci(eslint): disable eslint rules that are failing (#8372)
  chore: release next
  fix: replace "\n" to support Windows for tokens output (#8352)
  chore: release next
  ci(next): install deps before running npm scripts (#8370)
  feat: provide legacy CSS custom props for backwards compatibility (#8355)
  fix(input-date-picker): no longer emits redundant change event (#8341)
  fix(shell-panel): adds border at the start when slotted in `panel-end` (#8314)
  build(deps): update dependency sortablejs to v1.15.1 (#8364)
benelan pushed a commit that referenced this pull request Dec 19, 2023
🤖 I have created a release *beep* *boop*
---


<details><summary>@esri/calcite-design-tokens: 2.1.0</summary>

##
[2.1.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-design-tokens@2.0.0...@esri/calcite-design-tokens@2.1.0)
(2023-12-19)


### Features

* **font tokens:** Add font family fallbacks
([#8389](#8389))
([b2fd255](b2fd255))
* Update tokens for better documentation
([#8395](#8395))
([ff19630](ff19630))


### Bug Fixes

* **color tokens:** Fix errors in schema
([#8446](#8446))
([f36a90e](f36a90e))
* **color-context tokens:** Only apply when `.calcite-mode-auto` is
applied
([#8344](#8344))
([19de817](19de817))
* Replace "\n" to support Windows for tokens output
([#8352](#8352))
([02cf5d5](02cf5d5))
</details>

<details><summary>@esri/calcite-components: 2.1.0</summary>

##
[2.1.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@2.0.0...@esri/calcite-components@2.1.0)
(2023-12-19)


### Features

* Add `validation-message` and `validation-icon` attributes to form
components for new internal `calcite-input-message`
([#8305](#8305))
([a554cfd](a554cfd))
* **dropdown-item:** Add disabled support
([#8312](#8312))
([4c311c6](4c311c6))
* **list-item:** Add calciteListItemToggle event.
([#8433](#8433))
([1d2fa04](1d2fa04))
* **list:** Add `calciteListDragStart` and `calciteListDragEnd` events
for when a drag starts and ends.
([#8361](#8361))
([1314605](1314605))
* **list:** Add drag event details to `calciteListDragStart` and
`calciteListDragEnd` events
([#8438](#8438))
([e199c08](e199c08))
* Provide legacy CSS custom props for backwards compatibility
([#8355](#8355))
([b0f063e](b0f063e))


### Bug Fixes

* **action-menu:** Filter hidden or disabled actions via keyboard.
([#8336](#8336))
([11c0007](11c0007))
* **action-menu:** Fix closing action menu after a drag occurs
([#8339](#8339))
([dcb8548](dcb8548))
* **action-menu:** Keep internal popover open property in sync
([#8387](#8387))
([38dff7c](38dff7c))
* **dropdown-item:** Avoid hover/active styling when disabled
([#8398](#8398))
([35817dc](35817dc))
* **floating-ui:** Improve floating element performance
([#8409](#8409))
([4d8cfb8](4d8cfb8))
* **input-date-picker:** Ensure range input toggling is consistent
([#8414](#8414))
([cd92586](cd92586))
* **input-date-picker:** No longer emits redundant change event
([#8341](#8341))
([cd5b92b](cd5b92b))
* **input-date-picker:** Respect the numberingSystem property when
rendering the input
([#8383](#8383))
([395b538](395b538))
* **list-item:** Drag grid cell should be accessible via arrow keys.
([#8353](#8353))
([2718ab3](2718ab3))
* **menu-item:** Improve keyboard navigability when `href` populated
([#8408](#8408))
([5b44798](5b44798))
* **modal:** Ensure document overflow styles are properly restored when
multiple modals are closed/removed
([#8390](#8390))
([f2c6b09](f2c6b09))
* Replace "\n" to support Windows for tokens output
([#8352](#8352))
([02cf5d5](02cf5d5))
* **shell-panel:** Adds border at the start when slotted in `panel-end`
([#8314](#8314))
([2d1a1e2](2d1a1e2))
* **shell, shell-panel:** Support resizing shell panel when there is an
iframe slotted in shell content
([#8317](#8317))
([e0f69c9](e0f69c9))
* **stepper:** Emits `calciteStepperItemChange` event when switched to
first step
([#8422](#8422))
([508979f](508979f))
* **table-cell:** Fix background css variable
([#8439](#8439))
([9e5c59b](9e5c59b))
* **tab:** Prevent vertical scrollbar on content pane when the height of
outer elements are specified
([#8399](#8399))
([9e6d901](9e6d901))
* Use Stencil watchers instead of global attributes util
([#8407](#8407))
([c531d81](c531d81))


### Dependencies

* The following workspace dependencies were updated
  * devDependencies
    * @esri/calcite-design-tokens bumped from ^2.1.0-next.1 to ^2.1.0
</details>

<details><summary>@esri/calcite-components-angular: 2.1.0</summary>

##
[2.1.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-angular@2.0.0...@esri/calcite-components-angular@2.1.0)
(2023-12-19)


### Miscellaneous Chores

* **@esri/calcite-components-angular:** Synchronize undefined versions


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @esri/calcite-components bumped from ^2.1.0-next.12 to ^2.1.0
</details>

<details><summary>@esri/calcite-components-react: 2.1.0</summary>

##
[2.1.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@2.0.0...@esri/calcite-components-react@2.1.0)
(2023-12-19)


### Miscellaneous Chores

* **@esri/calcite-components-react:** Synchronize undefined versions


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @esri/calcite-components bumped from ^2.1.0-next.12 to ^2.1.0
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. p - high Issue should be addressed in the current milestone, impacts component or core functionality skip visual snapshots Pull requests that do not need visual regression testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants