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.

Sign up for GitHub

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

Jump to bottom

ui-trackoccupancydiagram: display trough trains #747

Merged
merged 3 commits into from
Dec 20, 2024
Merged

ui-trackoccupancydiagram: display trough trains #747

merged 3 commits into from
Dec 20, 2024

Conversation

Uriel-Sautron
Copy link
Contributor

@Uriel-Sautron Uriel-Sautron commented Dec 6, 2024 •
edited by Yohh
Loading

close #742
close #743
close #740

@Uriel-Sautron Uriel-Sautron requested a review from a team as a code owner December 6, 2024 15:51
@Uriel-Sautron Uriel-Sautron force-pushed the usn/ui-trackoccupancydiagram-display-trough-trains branch 2 times, most recently from c6a937d to 1ff27a1 Compare December 9, 2024 10:25
@Yohh Yohh force-pushed the usn/ui-trackoccupancydiagram-display-trough-trains branch 6 times, most recently from dac5a55 to 7bbc676 Compare December 17, 2024 15:21
@Yohh Yohh requested review from clarani and theocrsb December 17, 2024 15:21
@theocrsb
Copy link
Contributor

I can't change the selected train in the storybook

@Yohh Yohh self-assigned this Dec 18, 2024
@Yohh
Copy link
Contributor

Yohh commented Dec 18, 2024 •
edited
Loading

I can't change the selected train in the storybook

as I told on Element, setting another selectedTrainId will be part of the last PR, there is no state triggered on the story yet

edit: you can still change the const in the story

@theocrsb
Copy link
Contributor

I can't change the selected train in the storybook

as I told on Element, setting another selectedTrainId will be part of the last PR, there is no state triggered on the story yet

edit: you can still change the const in the story

Okay my bad, but otherwise all the acceptance criteria seem okay to me.
Just the hours poster seems a bit too high.

@Yohh
Copy link
Contributor

Yohh commented Dec 18, 2024 •
edited
Loading

Okay my bad, but otherwise all the acceptance criteria seem okay to me. Just the hours poster seems a bit too high.

on sketch the font size is set to 12px for the arrival and departure times, is it what you talk about? or the hours on the time-captions?

@theocrsb
Copy link
Contributor

Okay my bad, but otherwise all the acceptance criteria seem okay to me. Just the hours poster seems a bit too high.

on sketch the font size is set to 12px for the arrival and departure times, is it what you talk about? or the hours on the time-captions?

here:
-app:
image
-mockup:
image

@clarani clarani self-assigned this Dec 19, 2024
clarani
clarani reviewed Dec 19, 2024
View reviewed changes
@Yohh
Copy link
Contributor

Yohh commented Dec 19, 2024

@theocrsb the arrival and departure time top position have been fixed in the last fixup, good catch

@Yohh Yohh requested a review from clarani December 19, 2024 13:45
@Yohh Yohh force-pushed the usn/ui-trackoccupancydiagram-display-trough-trains branch 2 times, most recently from a914cb8 to 592b62c Compare December 20, 2024 07:56
theocrsb
theocrsb approved these changes Dec 20, 2024
View reviewed changes
Copy link
Contributor

@theocrsb theocrsb left a comment

Choose a reason for hiding this comment

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

LGTM and tested

@Yohh Yohh requested a review from clarani December 20, 2024 10:33
@Yohh Yohh force-pushed the usn/ui-trackoccupancydiagram-display-trough-trains branch 2 times, most recently from 0a8a06d to ed86fe6 Compare December 20, 2024 10:47
clarani
clarani reviewed Dec 20, 2024
View reviewed changes
Yohh and others added 3 commits December 20, 2024 16:59
@Yohh @Uriel-Sautron
ui-trackoccupancydiagram: add through train
8091219 
  - create drawThroughTrain and drawDefaultZone function

Co-authored-by: Uriel-Sautron <[email protected]>
Signed-off-by: Yohh <[email protected]>
@Yohh @Uriel-Sautron
ui-trackoccupancydiagram: display simultaneous occupancy zones
164e905 
Co-authored-by: Uriel-Sautron <[email protected]>
Signed-off-by: Yohh <[email protected]>
@Yohh @Uriel-Sautron
ui-trackoccupancydiagram: highlight selected train
d7b5903 
Co-authored-by: Uriel-Sautron <[email protected]>
Signed-off-by: Yohh <[email protected]>
@Yohh Yohh force-pushed the usn/ui-trackoccupancydiagram-display-trough-trains branch from 5c924e3 to d7b5903 Compare December 20, 2024 16:04
clarani
clarani approved these changes Dec 20, 2024
View reviewed changes
Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@Yohh Yohh enabled auto-merge December 20, 2024 16:06
@Yohh Yohh added this pull request to the merge queue Dec 20, 2024
Merged via the queue into dev with commit f77a5e2 Dec 20, 2024
6 checks passed
@Yohh Yohh deleted the usn/ui-trackoccupancydiagram-display-trough-trains branch December 20, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yohh Yohh Yohh left review comments

@clarani clarani clarani approved these changes

@theocrsb theocrsb theocrsb approved these changes

Assignees

@Yohh Yohh

@clarani clarani

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@Uriel-Sautron @theocrsb @Yohh @clarani