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

remove react css transition #10523

Merged
merged 2 commits into from
Jan 31, 2025
Merged

Conversation

SharglutDev
Copy link
Contributor

@SharglutDev SharglutDev commented Jan 24, 2025

See commits

Remove react-ccs-transition lib for buttons tooltip in favor of title html attribute (validated by @thibautsailly).
In the future, if the design system requires some custom tooltip, we can use the native popover api and anchor api (when it goes available on firefox).

fix #9595
fix #10524

To test :

  • check that the style for map buttons in all maps (reference, infra editor, manage train schedule, simulation results, stdcm config, stdcm results) is style working and that the title tooltip is showing when hovering the buttons
  • check for title tooltip in infra editor for all tools, waypoint selection in route tool
  • check for title tooltip for the info icon in incompatible constraints (in operational studies)
  • check for title tootip for the delete itinerary button (in operational studies)
  • check that there is no more warning in the console when hovering one of these buttons

@SharglutDev SharglutDev requested a review from a team as a code owner January 24, 2025 14:23
@github-actions github-actions bot added the area:front Work on Standard OSRD Interface modules label Jan 24, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 24, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 96.72131% with 4 lines in your changes missing coverage. Please review.

Project coverage is 81.83%. Comparing base (2e46b4b) to head (bad24fb).
Report is 2 commits behind head on dev.

Files with missing lines Patch % Lines
...ont/src/common/BootstrapSNCF/CheckboxRadioSNCF.tsx 0.00% 4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #10523      +/-   ##
==========================================
- Coverage   81.85%   81.83%   -0.02%     
==========================================
  Files        1075     1073       -2     
  Lines      107153   107058      -95     
  Branches      728      728              
==========================================
- Hits        87708    87610      -98     
- Misses      19406    19409       +3     
  Partials       39       39              
Flag Coverage Δ
editoast 74.24% <ø> (-0.01%) ⬇️
front 89.37% <96.72%> (-0.02%) ⬇️
gateway 2.18% <ø> (ø)
osrdyne 3.28% <ø> (ø)
railjson_generator 87.50% <ø> (ø)
tests 88.14% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Uriel-Sautron Uriel-Sautron left a comment

Choose a reason for hiding this comment

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

With the changes in the infra editor, we now have a vertical scroll bar and a horizontal scroll bar.

Copy link
Contributor

@Uriel-Sautron Uriel-Sautron left a comment

Choose a reason for hiding this comment

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

LGTM, tested !!!

@SharglutDev SharglutDev force-pushed the pfn/front/remove-react-css-transition branch from cd2debb to 5e90eac Compare January 31, 2025 11:12
Copy link
Contributor

@Uriel-Sautron Uriel-Sautron left a comment

Choose a reason for hiding this comment

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

Tested

Copy link
Contributor

@kmer2016 kmer2016 left a comment

Choose a reason for hiding this comment

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

LGTM ! thank you for cleaning !

This library didn't have a new release since 2022. It uses a react tool that will be deprecated in react v19 (findDOMNode).
It was used to display tooltips when hovering some buttons in the app.
Since the design system doesn't have a custom tooltip for buttons, it has been requested to just use the regular title html attribute.
Instead of finding a substitute lib, just remove the old one and use the title attribute.

Signed-off-by: SharglutDev <[email protected]>
@SharglutDev SharglutDev force-pushed the pfn/front/remove-react-css-transition branch from 5e90eac to bad24fb Compare January 31, 2025 16:22
@SharglutDev SharglutDev enabled auto-merge January 31, 2025 16:23
@SharglutDev SharglutDev added this pull request to the merge queue Jan 31, 2025
Merged via the queue into dev with commit 586b13c Jan 31, 2025
27 checks passed
@SharglutDev SharglutDev deleted the pfn/front/remove-react-css-transition branch January 31, 2025 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:front Work on Standard OSRD Interface modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wrong tooltip for waypoint selection in route tool React warning in maps
4 participants