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

fix: timetableGridItem title maxLine3 while keeping the existing layout #917

Merged
merged 4 commits into from
Sep 3, 2024
Merged

fix: timetableGridItem title maxLine3 while keeping the existing layout #917

merged 4 commits into from
Sep 3, 2024
+90 −34

Conversation

Matuyuhi
Copy link
Contributor

@Matuyuhi Matuyuhi commented Sep 1, 2024

Issue

Overview (Required)

  • This PR fixes the timelineText component to ensure it is limited to a maximum of three lines while preserving the existing layout.

Links

Screenshot (Optional if screenshot test is present or unrelated to UI)

Before After

@github-actions github-actions bot temporarily deployed to deploygate-distribution September 1, 2024 12:53 Inactive
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 1, 2024 •
edited
Loading

Snapshot diff report

File name Image
TimetableVerticalSca
le60PercentPreview_c
ompare.png
TimetableVerticalSca
le20PercentPreview_c
ompare.png
PreviewTimetableGrid
ItemMoreLongTitleIte
m_compare.png
TimetableVerticalSca
le40PercentPreview_c
ompare.png

@Matuyuhi
Copy link
Contributor Author

Matuyuhi commented Sep 1, 2024 •
edited
Loading

↑ Fix padding between texts
8c6f0e7

@Matuyuhi Matuyuhi mentioned this pull request Sep 1, 2024
Closed
@takahirom
Copy link
Member

takahirom commented Sep 1, 2024 •
edited
Loading

Looks great! Could we have screenshot tests so we can check the three lines and more of the sessions?

@github-actions github-actions bot temporarily deployed to deploygate-distribution September 1, 2024 13:37 Inactive
@Matuyuhi
Copy link
Contributor Author

Matuyuhi commented Sep 1, 2024

Thanks! I'll wait for the running test as it is.

@Matuyuhi
Copy link
Contributor Author

Matuyuhi commented Sep 1, 2024

Added preview of long titles!

@Matuyuhi Matuyuhi force-pushed the companion_fix/issue-902 branch from e872e43 to ab422d8 Compare September 1, 2024 15:09
@github-actions github-actions bot temporarily deployed to deploygate-distribution September 1, 2024 15:20 Inactive
@takahirom
Copy link
Member

I think it might be better for this pattern to be on one line because the space between the title and the speaker icon is a little narrow.
image

@Matuyuhi
Copy link
Contributor Author

Matuyuhi commented Sep 2, 2024

good idea. I would put the same spacing between the title and the speaker icon as the schedule.

@takahirom
Copy link
Member

Thanks!

@Matuyuhi
Copy link
Contributor Author

Matuyuhi commented Sep 2, 2024

Ready to tests!

@github-actions github-actions bot temporarily deployed to deploygate-distribution September 2, 2024 23:55 Inactive
takahirom
takahirom approved these changes Sep 3, 2024
View reviewed changes
Copy link
Member

@takahirom takahirom left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for fixing this difficult layout.

@takahirom takahirom merged commit 48e292d into DroidKaigi:main Sep 3, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@takahirom takahirom takahirom approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TimeTableGridItem title text line count is different from Figma
2 participants
@Matuyuhi @takahirom