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

Use CSS Logical Properties, remove dir usage #2079

Closed
45 tasks done
caripizza opened this issue Apr 29, 2021 · 16 comments
Closed
45 tasks done

Use CSS Logical Properties, remove dir usage #2079

caripizza opened this issue Apr 29, 2021 · 16 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. epic Large scale issues to be broken up into sub-issues and tracked via sprints and/or project. refactor Issues tied to code that needs to be significantly reworked.

Comments

@caripizza
Copy link
Contributor

caripizza commented Apr 29, 2021

Summary
Refactor component styles to use CSS Logical Properties and Values, perhaps via Tailwind v2 plugin

Start using flow-relative CSS values like start/end (This would get rid of 99% of > the use for getElementDir())

Relevant Info

Components

  • calcite-accordion-item/calcite-accordion-item.tsx
  • calcite-action-pad/calcite-action-pad.tsx
  • calcite-alert/calcite-alert.tsx
  • calcite-avatar/calcite-avatar.tsx
  • calcite-button/calcite-button.tsx
  • calcite-card/calcite-card.tsx
  • calcite-chip/calcite-chip.tsx
  • calcite-color-picker/calcite-color-picker.tsx
  • calcite-color-picker-hex-input/calcite-color-picker-hex-input.tsx
  • calcite-combobox/calcite-combobox.tsx
  • calcite-combobox-item/calcite-combobox-item.tsx
  • calcite-combobox-item-group/calcite-combobox-item-group.tsx
  • calcite-date-picker/calcite-date-picker.tsx
  • calcite-date-picker-day/calcite-date-picker-day.tsx
  • calcite-date-picker-month/calcite-date-picker-month.tsx
  • calcite-date-picker-month-header/calcite-date-picker-month-header.tsx
  • calcite-dropdown/calcite-dropdown.tsx
  • calcite-dropdown-group/calcite-dropdown-group.tsx
  • calcite-dropdown-item/calcite-dropdown-item.tsx
  • calcite-fab/calcite-fab.tsx
  • calcite-filter/calcite-filter.tsx
  • calcite-icon/calcite-icon.tsx
  • calcite-input/calcite-input.tsx
  • calcite-input-date-picker/calcite-input-date-picker.tsx
  • calcite-input-message/calcite-input-message.tsx
  • calcite-label/calcite-label.tsx
  • calcite-link/calcite-link.tsx
  • calcite-modal/calcite-modal.tsx
  • calcite-notice/calcite-notice.tsx
  • calcite-pagination/calcite-pagination.tsx
  • calcite-panel/calcite-panel.tsx
  • calcite-pick-list/shared-list-render.tsx
  • calcite-pick-list-group/calcite-pick-list-group.tsx
  • calcite-popover/calcite-popover.tsx
  • calcite-radio-group-item/calcite-radio-group-item.tsx
  • calcite-rating/calcite-rating.tsx
  • calcite-select/calcite-select.tsx
  • calcite-shell-center-row/calcite-shell-center-row.tsx
  • calcite-split-button/calcite-split-button.tsx - has props to specify whether icon should flip in RTL contexts
  • calcite-stepper-item/calcite-stepper-item.tsx
  • calcite-switch/calcite-switch.tsx
  • calcite-tab-title/calcite-tab-title.tsx
  • calcite-tile-select/calcite-tile-select.tsx
  • calcite-tree-item/calcite-tree-item.tsx
  • functional/CalciteExpandToggle.tsx
@caripizza caripizza added refactor Issues tied to code that needs to be significantly reworked. 0 - new New issues that need assignment. epic Large scale issues to be broken up into sub-issues and tracked via sprints and/or project. labels Apr 29, 2021
@benelan benelan added the needs triage Planning workflow - pending design/dev review. label Sep 27, 2021
@driskull
Copy link
Member

I think we should bump the priority on this @jcfranco @julio8a.

We can proceed with this without upgrading to tailwind2 or using a tailwind plugin I think?

@jcfranco
Copy link
Member

jcfranco commented Oct 1, 2021

Yeah, I think we need to tackle the logical props part before 1.0.0. cc @julio8a @benelan

#2868 is sort of related to this, right?

@benelan
Copy link
Member

benelan commented Oct 1, 2021

I agree about tackling logical props before 1.0.0 but I'm not sure the cascading scale thing is related. @jcfranco

@jcfranco
Copy link
Member

jcfranco commented Oct 1, 2021

Gotcha. Never mind then. I was under the impression that logical properties would also work through the cascade, but I can't find any examples.

@caripizza Can this be done with our current version of Tailwind? I see that tailwindcss-logical is compatible w/ 1.2.0+.

@driskull
Copy link
Member

driskull commented Oct 1, 2021

In some places, we're already using logical properties without tailwind. https://github.com/Esri/calcite-components/blob/master/src/components/calcite-radio-group-item/calcite-radio-group-item.scss#L81-L107

@jcfranco
Copy link
Member

jcfranco commented Oct 1, 2021

@driskull Thanks, that's good to know. FWIW, I want to determine if we need to split this up into another issue or if we can tackle this one as is.

@benelan benelan removed the needs triage Planning workflow - pending design/dev review. label Oct 1, 2021
@caripizza
Copy link
Contributor Author

Yep I've just been using manual margin-inline styles since they're not built into TW. The plugin could make sense down the road but I'm not sure it's necessary at the moment.

@jcfranco
Copy link
Member

jcfranco commented Oct 6, 2021

Thanks for the info, @caripizza.

In that case, let's create a separate issue to go ahead with the CSS logical properties work and repurpose this as a refactor issue. Does that sound alright? cc @benelan

@eriklharper
Copy link
Contributor

Looks like it will be beneficial for me to convert button and action to use CSS logical properties so that the new snapshot visual tests will pass in support of #3140. I'm not sure if we want to use #2079 (this issue) to track all the components that need converted or not, but I thought I would alert others following this issue that I plan on tackling this for these two components even though there's not a specific issue to address doing this for button and action.

@driskull
Copy link
Member

driskull commented Oct 6, 2021

any component using getElementDir() needs to be converted and that utility method removed once all instances are no longer needed.

@driskull
Copy link
Member

driskull commented Dec 3, 2021

@benelan @jcfranco can we prioritize this for a sprint?

It would include...

  • Replacing CSS with logical css properties instead of getElementDir usage
  • Removing use of rtl css utiity

@eriklharper
Copy link
Contributor

Hoo boy that's a lot!

@driskull driskull changed the title Tailwind: use CSS Logical Properties, remove dir usage Use CSS Logical Properties, remove dir usage Dec 3, 2021
@jcfranco jcfranco added this to the Sprint 12/6 - 12/17 milestone Dec 3, 2021
@jcfranco
Copy link
Member

jcfranco commented Dec 3, 2021

@driskull Moved to the upcoming sprint. I can help with this early next sprint. Let me know how you want to divvy it up.

@driskull
Copy link
Member

driskull commented Dec 7, 2021

@jcfranco you want to just start from the bottom of the list? I've started from the top.

driskull added a commit that referenced this issue Dec 10, 2021
* refactor: Remove setting 'dir' attribute within components. #2079

* fix styles

* fix screener?

* cleanup

* revert link changes

* fix link

* fix merge issue

* alert, action-pad.

* button

* accordion-item

* card

* chip

* color-picker, popper

* combobox

* combobox-item

* combobox-item-group

* popper test fix, dropdown

* filter

* fix test?

* icon

* alert, dropdown-item, link

* input

* input-message

* label

* modal

* notice

* move tile-select to use css logical props

* update tab-title to use css logical props

* update switch to use css logical props

* update stepper-item to use css logical props

* panel

* pick-list-group, pick-list-item

* block-section

* popover

* tree-item

* rating

* update select use css logical props

* update shell-center-row to use css logical props

* fix select styles

* refactor(filter): enforce items prop via type (#3643)

* fix(action-bar): Allow slotting tooltip after initialization (#3642)

* fix(action-bar): Allow slotting tooltip after initialization.  #3495

* review fixes

* build(deps): bump @storybook/cli from 6.4.1 to 6.4.8 (#3638)

Bumps [@storybook/cli](https://github.com/storybookjs/storybook/tree/HEAD/lib/cli) from 6.4.1 to 6.4.8.
- [Release notes](https://github.com/storybookjs/storybook/releases)
- [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md)
- [Commits](https://github.com/storybookjs/storybook/commits/v6.4.8/lib/cli)

---
updated-dependencies:
- dependency-name: "@storybook/cli"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* 1.0.0-next.318

* build(deps): bump eslint from 8.3.0 to 8.4.1 (#3649)

Bumps [eslint](https://github.com/eslint/eslint) from 8.3.0 to 8.4.1.
- [Release notes](https://github.com/eslint/eslint/releases)
- [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md)
- [Commits](eslint/eslint@v8.3.0...v8.4.1)

---
updated-dependencies:
- dependency-name: eslint
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump @esri/calcite-ui-icons from 3.17.6 to 3.17.8 (#3650)

Bumps [@esri/calcite-ui-icons](https://github.com/Esri/calcite-ui-icons) from 3.17.6 to 3.17.8.
- [Release notes](https://github.com/Esri/calcite-ui-icons/releases)
- [Commits](Esri/calcite-ui-icons@v3.17.6...v3.17.8)

---
updated-dependencies:
- dependency-name: "@esri/calcite-ui-icons"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump @storybook/addon-docs from 6.4.1 to 6.4.8 (#3648)

Bumps [@storybook/addon-docs](https://github.com/storybookjs/storybook/tree/HEAD/addons/docs) from 6.4.1 to 6.4.8.
- [Release notes](https://github.com/storybookjs/storybook/releases)
- [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md)
- [Commits](https://github.com/storybookjs/storybook/commits/v6.4.8/addons/docs)

---
updated-dependencies:
- dependency-name: "@storybook/addon-docs"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump color from 4.0.2 to 4.1.0 (#3656)

Bumps [color](https://github.com/Qix-/color) from 4.0.2 to 4.1.0.
- [Release notes](https://github.com/Qix-/color/releases)
- [Commits](Qix-/color@4.0.2...4.1.0)

---
updated-dependencies:
- dependency-name: color
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump @typescript-eslint/parser from 5.5.0 to 5.6.0 (#3655)

Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 5.5.0 to 5.6.0.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.6.0/packages/parser)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/parser"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump @storybook/addon-a11y from 6.4.1 to 6.4.8 (#3654)

Bumps [@storybook/addon-a11y](https://github.com/storybookjs/storybook/tree/HEAD/addons/a11y) from 6.4.1 to 6.4.8.
- [Release notes](https://github.com/storybookjs/storybook/releases)
- [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md)
- [Commits](https://github.com/storybookjs/storybook/commits/v6.4.8/addons/a11y)

---
updated-dependencies:
- dependency-name: "@storybook/addon-a11y"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* docs: update component READMEs (#3652)

Co-authored-by: jcfranco <[email protected]>

* build(deps): bump ts-jest from 27.0.7 to 27.1.0 (#3647)

Bumps [ts-jest](https://github.com/kulshekhar/ts-jest) from 27.0.7 to 27.1.0.
- [Release notes](https://github.com/kulshekhar/ts-jest/releases)
- [Changelog](https://github.com/kulshekhar/ts-jest/blob/main/CHANGELOG.md)
- [Commits](kulshekhar/ts-jest@v27.0.7...v27.1.0)

---
updated-dependencies:
- dependency-name: ts-jest
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump @storybook/html from 6.4.1 to 6.4.8 (#3641)

Bumps [@storybook/html](https://github.com/storybookjs/storybook/tree/HEAD/app/html) from 6.4.1 to 6.4.8.
- [Release notes](https://github.com/storybookjs/storybook/releases)
- [Changelog](https://github.com/storybookjs/storybook/blob/next/CHANGELOG.md)
- [Commits](https://github.com/storybookjs/storybook/commits/v6.4.8/app/html)

---
updated-dependencies:
- dependency-name: "@storybook/html"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump prettier from 2.5.0 to 2.5.1 (#3658)

Bumps [prettier](https://github.com/prettier/prettier) from 2.5.0 to 2.5.1.
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md)
- [Commits](prettier/prettier@2.5.0...2.5.1)

---
updated-dependencies:
- dependency-name: prettier
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(panel): the panel is now dismissed on escape keydown and not keyup for consistency (#3657)

Co-authored-by: Eliza Khachatryan <[email protected]>

* 1.0.0-next.319

* feat(input-time-picker)!: localization support for input-time-picker (#3354)

* chore: fix linting issues (#3581)

* 1.0.0-next.320

* make sure all values are grabbed from tailwind theme

* first pass at date-picker-day using logical props

* tidy up and add missing text logical prop

* fix test

* cleanup

* remove getElementDir

* cleanup stepper, switch

* fix casing

* fix link animation

* clean up conventions

* use consistent casing for default value

* tidy up

* review fixes

* fix alert padding.

* fix notice

* screener test

* Revert "screener test"

This reverts commit 74c0233.

* fix anim

* revert link change

* fix overlapping border styles from date-picker-day

* fix test

* fix story

* fix test

* split up border radius for range start/ends

* simplify border radius?

* put the 45% radius back.

* revert

Co-authored-by: JC Franco <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Ben Elan <[email protected]>
Co-authored-by: jcfranco <[email protected]>
Co-authored-by: Eliza Khachatryan <[email protected]>
Co-authored-by: Eliza Khachatryan <[email protected]>
Co-authored-by: Erik Harper <[email protected]>
Co-authored-by: Duc Phan <[email protected]>
@driskull driskull removed the 0 - new New issues that need assignment. label Dec 13, 2021
@driskull driskull added the 3 - installed Issues that have been merged to master branch and are ready for final confirmation. label Dec 13, 2021
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@benelan
Copy link
Member

benelan commented Dec 29, 2021

This has been installed for two weeks and there have not been any reports of visual regressions or unintended changes. Closing as verified!

@benelan benelan closed this as completed Dec 29, 2021
@benelan benelan added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. epic Large scale issues to be broken up into sub-issues and tracked via sprints and/or project. refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Development

No branches or pull requests

5 participants