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

[Link] Incorrect underline animation in RTL #7284

Closed
2 of 3 tasks
macandcheese opened this issue Jul 5, 2023 · 14 comments
Closed
2 of 3 tasks

[Link] Incorrect underline animation in RTL #7284

macandcheese opened this issue Jul 5, 2023 · 14 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Map Viewer Issues logged by ArcGIS Map Viewer team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. figma changes Issues that require additions or updates to the Figma UI Kit where no `design` label exists good first issue Issues that can be worked on by contributors new to calcite-components. i18n-l10n issues dealing with internationalization/localization impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone p - low Issue is non core or affecting less that 10% of people using the library

Comments

@macandcheese
Copy link
Contributor

macandcheese commented Jul 5, 2023

Check existing issues

Actual Behavior

The underline animation does not correctly animate in the expected direction when in RTL.

Expected Behavior

I'd expect the animation to correctly display as it does in LTR, but flipped.

cc @SkyeSeitz @ashetland - I know we've talked about changing this animation altogether and favoring something less... busy - maybe an additional px height on hover, just color change, etc., so could be a good opportunity to look at that.

Related: #6569

Reproduction Sample

https://codepen.io/mac_and_cheese/pen/qBQXYKB?editors=1000

Reproduction Steps

  1. Go to Codepen
  2. See difference in animation

Reproduction Version

Latest

Relevant Info

No response

Regression?

No response

Priority impact

p2 - want for current milestone

Impact

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-react

Esri team

ArcGIS Map Viewer

@macandcheese macandcheese added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Jul 5, 2023
@github-actions github-actions bot added impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone ArcGIS Map Viewer Issues logged by ArcGIS Map Viewer team members. calcite-components Issues specific to the @esri/calcite-components package. labels Jul 5, 2023
@annierm18 annierm18 added the i18n-l10n issues dealing with internationalization/localization label Jul 18, 2023
@geospatialem geospatialem added p - low Issue is non core or affecting less that 10% of people using the library estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. design Issues that need design consultation prior to development. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. and removed needs triage Planning workflow - pending design/dev review. labels Aug 7, 2023
@geospatialem
Copy link
Member

cc @ashetland @SkyeSeitz WDYT/ any thoughts on changes to the link animation?

@SkyeSeitz
Copy link

I could definitely see a few options where the animation is removed. I think I lean towards just an instant color change from 40% opacity to 100%, as seen in the prototype below.

Screen.Recording.2023-08-07.at.11.53.34.AM.mov

@ashetland
Copy link
Contributor

I like that instant change. Mocked up a bump from 1px to 2px for the indicator @macandcheese mentioned just for comparison. I think I go for the simple version or just instant timing.
CleanShot 2023-08-07 at 12 09 13

@SkyeSeitz
Copy link

Clarifying question for @macandcheese 😅 - Additional px to the underline or the height of the link? Prototype below to demonstrate:

Screen.Recording.2023-08-07.at.12.14.12.PM.mov

@brittneytewks brittneytewks added the figma changes Issues that require additions or updates to the Figma UI Kit where no `design` label exists label Sep 12, 2023
@geospatialem geospatialem added this to the 2024 March Priorities milestone Nov 22, 2023
@geospatialem geospatialem removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Nov 22, 2023
@brittneytewks brittneytewks removed the figma changes Issues that require additions or updates to the Figma UI Kit where no `design` label exists label Dec 11, 2023
@jcfranco jcfranco added the good first issue Issues that can be worked on by contributors new to calcite-components. label Feb 20, 2024
@brittneytewks brittneytewks removed the design Issues that need design consultation prior to development. label Mar 4, 2024
@brittneytewks brittneytewks added the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Mar 25, 2024
@brittneytewks brittneytewks removed this from the 2024-04-30 - Apr Release milestone Mar 25, 2024
@brittneytewks brittneytewks added the figma changes Issues that require additions or updates to the Figma UI Kit where no `design` label exists label Mar 27, 2024
@geospatialem geospatialem added this to the 2024-11-19 - Nov Release milestone Apr 3, 2024
@geospatialem geospatialem removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Apr 3, 2024
@jcfranco
Copy link
Member

@macandcheese Could you help clarify the comment above?

@macandcheese
Copy link
Contributor Author

Sorry, which comment 😅

I think the immediate issue can be fixed with no design change, and then we can separately discuss an update there cc @SkyeSeitz @ashetland (I wouldn't think that a border change bumps the height up like in that example)

@jcfranco
Copy link
Member

@macandcheese I was referring to #7284 (comment), apologies for not specifying earlier.

One more question, are we keeping the animation as is, but inverting it or are we also simplifying it to just color change?

@macandcheese
Copy link
Contributor Author

macandcheese commented Nov 21, 2024

The scope of the issue initially was intended to just fix the RTL implementation by inverting it. It works fine in LTR. I think this other discussion is good and we should update the visual, but can be moved to a separate design issue?

@ashetland
Copy link
Contributor

Agree, we can move that to a separate design issue. Is the animation working correctly in LTR? I'm not seeing it here https://developers.arcgis.com/calcite-design-system/components/link/

@jcfranco
Copy link
Member

Looks like the animation has been broken since v2.7.0 (possibly by #8797).

@macandcheese
Copy link
Contributor Author

Heh - well in this case it's a "fix" as it behaves the same in both. I don't think the animation really adds anything. Maybe we can remove the code that isn't working and use another issue to track improvements. I'd think anything that lets us get rid of the "transparent underline" css, etc. could be nice.

@ashetland
Copy link
Contributor

Agreed

@jcfranco jcfranco self-assigned this Nov 22, 2024
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. labels Nov 22, 2024
jcfranco added a commit that referenced this issue Nov 22, 2024
**Related Issue:** #7284

## Summary

Restores the underline animation that was broken in v2.7.0
(#8797) and updates it
to work consistently in both LTR/RTL directions.

This also adds a `transition-default` mixin to provide finer-grained
control of transitions.
@jcfranco jcfranco added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Nov 22, 2024
Copy link
Contributor

Installed and assigned for verification.

@github-actions github-actions bot assigned ashetland and brittneytewks and unassigned jcfranco Nov 22, 2024
@DitwanP
Copy link
Contributor

DitwanP commented Nov 22, 2024

Verified locally on dev

Fixed animations:

Screen.Recording.2024-11-22.at.11.26.44.AM.mov

@DitwanP DitwanP closed this as completed Nov 22, 2024
@DitwanP DitwanP 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 Nov 22, 2024
benelan pushed a commit that referenced this issue Feb 8, 2025
**Related Issue:** #7284

## Summary

Restores the underline animation that was broken in v2.7.0
(#8797) and updates it
to work consistently in both LTR/RTL directions.

This also adds a `transition-default` mixin to provide finer-grained
control of transitions.
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. ArcGIS Map Viewer Issues logged by ArcGIS Map Viewer team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. estimate - 1 Very small fix or change (potentially a single line), doesn't require updates to tests. figma changes Issues that require additions or updates to the Figma UI Kit where no `design` label exists good first issue Issues that can be worked on by contributors new to calcite-components. i18n-l10n issues dealing with internationalization/localization impact - p2 - want for an upcoming milestone User set priority impact status of p2 - want for an upcoming milestone p - low Issue is non core or affecting less that 10% of people using the library
Projects
None yet
Development

No branches or pull requests

8 participants