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: fix scroll in stdcm result table #10274

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

Conversation

RomainValls
Copy link
Contributor

close #9625

Decided to put a max height of 80vh to prevent the results from increasing page size too much.
Can be changed for lesser / higher value if you think it's better to.

@RomainValls RomainValls requested a review from a team as a code owner January 7, 2025 16:50
@github-actions github-actions bot added the area:front Work on Standard OSRD Interface modules label Jan 7, 2025
@RomainValls RomainValls requested review from kmer2016 and Yohh January 7, 2025 16:51
@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2025

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

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.50%. Comparing base (ef6e28e) to head (3cb2c46).
Report is 17 commits behind head on dev.

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

Additional details and impacted files
@@             Coverage Diff             @@
##              dev   #10274       +/-   ##
===========================================
+ Coverage   81.45%   87.50%    +6.05%     
===========================================
  Files        1058       31     -1027     
  Lines      104235     1537   -102698     
  Branches      722        0      -722     
===========================================
- Hits        84907     1345    -83562     
+ Misses      19287      192    -19095     
+ Partials       41        0       -41     
Flag Coverage Δ
editoast ?
front ?
gateway ?
osrdyne ?
railjson_generator 87.50% <ø> (ø)
tests ?

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.

@RomainValls RomainValls force-pushed the rvs/front-fix-overflow-in-stdcm-result-table branch from f26cd69 to e0b28c5 Compare January 7, 2025 17:17
Copy link
Contributor

@Yohh Yohh left a comment

Choose a reason for hiding this comment

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

seems ok but just a suggestion:

to fit on screens, you could set a height on .stdcm__body:

    height: calc(100vh - 120px);

(100vh - stdcm navbar)

then remove your max-height and set a height on .simulation-results:

    height: calc(100vh - 180px);

(100vh - y-paddings - simulation-list height)

this way the should always fit on every screen heights

can you try it?
@kmer2016 do you agree?

@kmer2016
Copy link
Contributor

kmer2016 commented Jan 8, 2025

seems ok but just a suggestion:

to fit on screens, you could set a height on .stdcm__body:

    height: calc(100vh - 120px);

(100vh - stdcm navbar)

then remove your max-height and set a height on .simulation-results:

    height: calc(100vh - 180px);

(100vh - y-paddings - simulation-list height)

this way the should always fit on every screen heights

can you try it? @kmer2016 do you agree?

I agree with the idea of making the height dynamic to improve responsiveness. However, after testing this approach, it slightly breaks the current layout.
Based on your suggestion, I propose the following

.table-container {
    height: calc(100vh - 120px);
    overflow-y: auto;
}

@RomainValls
Copy link
Contributor Author

Thank you for the suggestions @Yohh and @kmer2016 🙏

@RomainValls RomainValls requested a review from Yohh January 8, 2025 16:05
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.

Scrolling in stdcm result scrolls through all waypoints even if not on the waypoints
4 participants