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

front: redesign of the list of trains on the results page #8765

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

theocrsb
Copy link
Contributor

@theocrsb theocrsb commented Sep 5, 2024

@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2024

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

Codecov Report

Attention: Patch coverage is 0% with 319 lines in your changes missing coverage. Please review.

Project coverage is 37.13%. Comparing base (3460672) to head (312d77a).
Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
...hedule/components/Timetable/TimetableTrainCard.tsx 0.00% 120 Missing ⚠️
...schedule/components/Timetable/TimetableToolbar.tsx 0.00% 105 Missing ⚠️
...trainschedule/components/Timetable/FilterPanel.tsx 0.00% 70 Missing and 1 partial ⚠️
...s/trainschedule/components/Timetable/Timetable.tsx 0.00% 21 Missing and 1 partial ⚠️
...tudies/components/Scenario/ScenarioDescription.tsx 0.00% 0 Missing and 1 partial ⚠️

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

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #8765      +/-   ##
============================================
+ Coverage     37.05%   37.13%   +0.07%     
  Complexity     2211     2211              
============================================
  Files          1260     1260              
  Lines        114820   114899      +79     
  Branches       3223     3223              
============================================
+ Hits          42551    42672     +121     
+ Misses        70337    70295      -42     
  Partials       1932     1932              
Flag Coverage Δ
core 74.79% <ø> (ø)
editoast 72.53% <ø> (+0.07%) ⬆️
front 15.21% <0.00%> (+0.04%) ⬆️
gateway 2.20% <ø> (ø)
osrdyne 2.60% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 86.46% <ø> (ø)

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.

@theocrsb theocrsb force-pushed the tce/front/trainschedule/redign-list-trains branch 18 times, most recently from c41c644 to fa398c7 Compare September 12, 2024 09:01
@theocrsb theocrsb marked this pull request as ready for review September 12, 2024 09:04
@theocrsb theocrsb requested a review from a team as a code owner September 12, 2024 09:04
@emersion emersion self-assigned this Sep 12, 2024
@theocrsb
Copy link
Contributor Author

@theocrsb theocrsb force-pushed the tce/front/trainschedule/redign-list-trains branch 2 times, most recently from 46341a2 to a809797 Compare September 13, 2024 07:03
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.

"Great work! Not much to say on the code side, just a small CSS suggestion. However, I have a few questions regarding the behavior:

  • It seems like a third scrollbar has appeared.
    image

  • When clicking the checkbox on a TrainCard, the corresponding train gets selected, and the curves and output tables are updated with that train's info. Previously, the train was selected by clicking on the TrainCard itself, not the checkbox.

  • In a new scenario, the 'No train' checkbox is always checked and cannot be unchecked.

  • There's no longer any distinction for trains on which path projection has been performed.

  • The hover effect on the first action button doesn't turn white like the others."

@theocrsb
Copy link
Contributor Author

"Great work! Not much to say on the code side, just a small CSS suggestion. However, I have a few questions regarding the behavior:

  • It seems like a third scrollbar has appeared.
    image
  • When clicking the checkbox on a TrainCard, the corresponding train gets selected, and the curves and output tables are updated with that train's info. Previously, the train was selected by clicking on the TrainCard itself, not the checkbox.
  • In a new scenario, the 'No train' checkbox is always checked and cannot be unchecked.
  • There's no longer any distinction for trains on which path projection has been performed.
  • The hover effect on the first action button doesn't turn white like the others."

This is done for the hover and scroll bar.
When there is no train, it is normal for the box to be checked.
For trains on which a path projection has been carried out, @thibautsailly will be able to give us a new display.
We'll need to solve the checkbox problem.

@theocrsb
Copy link
Contributor Author

I pushed a solution for the checkbox problem

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 !

@emersion emersion force-pushed the tce/front/trainschedule/redign-list-trains branch 3 times, most recently from 312d77a to 02f9614 Compare September 18, 2024 08:46
@emersion emersion requested a review from a team as a code owner September 18, 2024 08:46
theocrsb and others added 2 commits September 18, 2024 10:47
Co-authored-by: Simon Ser <[email protected]>
Signed-off-by: theocrsb <[email protected]>
Signed-off-by: Simon Ser <[email protected]>
Co-authored-by: Simon Ser <[email protected]>
Signed-off-by: theocrsb <[email protected]>
Signed-off-by: Simon Ser <[email protected]>
@emersion emersion force-pushed the tce/front/trainschedule/redign-list-trains branch from 02f9614 to 559b79c Compare September 18, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redesign of the list of trains on the results page
4 participants