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

small-and-fast: make bar-graph table responsive #1907

Merged
merged 2 commits into from
Oct 18, 2024
Merged

small-and-fast: make bar-graph table responsive #1907

merged 2 commits into from
Oct 18, 2024

Conversation

To1ne
Copy link
Contributor

@To1ne To1ne commented Oct 16, 2024

The bar charts are shown in a table. This doesn't work well when viewed
on mobile, because the screen is a lot narrower than the content.

Use CSS grid to responsively show bar charts on a single line depending
on the available horizontal space.

Screenshots

Screen width: 1200px

Screen Shot 2024-10-16 at 16 13 18

Screen width: 939px (just below 940px breakproint where sidebar is hidden)

Screen Shot 2024-10-16 at 16 17 15

Screen width: 877px

Screen Shot 2024-10-16 at 16 17 48

Screen width: 876px

Screen Shot 2024-10-16 at 16 18 32

Screen width: 762px

Screen Shot 2024-10-16 at 16 19 04

Screen width: 761px

Screen Shot 2024-10-16 at 16 20 02

Screen width: 647px

Screen Shot 2024-10-16 at 16 20 49

Screen width: 600px

Screen Shot 2024-10-16 at 16 21 27

Screen width: 375px (iPhone 13mini according to web inspector)

Screen Shot 2024-10-16 at 16 22 02

@To1ne
sass: move about styling into separate file
4035aad 
assets/sass/application.scss is the main entry point for all sub
stylesheets. Recently we've added `.bar-chart` to that file, while it's
only used on one of the /about pages.

We're about to add more styling to the /about pages, so split out it's
styling into a separate file.
@To1ne
Copy link
Contributor Author

To1ne commented Oct 16, 2024

I've tweaked the values a bit, so there are max 6 graphs per line. Screenshots are thus not fully up-to-date.

@To1ne
Copy link
Contributor Author

To1ne commented Oct 17, 2024

@dscho it's now deployed to https://to1ne.github.io/git-scm.com/about/small-and-fast.

@To1ne
Copy link
Contributor Author

To1ne commented Oct 17, 2024 •
edited
Loading

:doh: I broke the circles:

EDIT: It's just a GitHub pages thingie being broken due to me used to having a custom domain.

image

dscho
dscho approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@dscho dscho 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 @To1ne!

How about adding a Playwright test to verify that the formerly-right-most Clone diagram is visible even on an iPhone (e.g. after the side-button tests)?

@To1ne
Copy link
Contributor Author

To1ne commented Oct 17, 2024 •
edited
Loading

How about adding a Playwright test to verify that the formerly-right-most Clone diagram is visible even on an iPhone (e.g. after the side-button tests)?

@dscho Well, I'm failing to figure how to use that. npx playwright install starts downloading a full copy of Chrome, that's not what I want. And npx playwright install-deps firefox ask for root access, that's also fishy.

(It doesn't help I really dislike Javascript and the whole ecosystem around it)

@To1ne
small-and-fast: make bar-graph table responsive
77d0c63 
The bar charts are shown in a table. This doesn't work well when viewed
on mobile, because the screen is a lot narrower than the content.

Use CSS grid to responsively show bar charts on a single line depending
on the available horizontal space.

This change includes a Playwright test that opens the page on an iPhone
X and scrolls to the text right below the graphs to see if the last
graph then is within the viewport.
@To1ne
Copy link
Contributor Author

To1ne commented Oct 17, 2024

@dscho Playwright test added.

@dscho
Copy link
Member

dscho commented Oct 18, 2024

I'm failing to figure how to use that. npx playwright install starts downloading a full copy of Chrome, that's not what I want. And npx playwright install-deps firefox ask for root access, that's also fishy.

Hrm. I think what you need to do is to run npx playwright test --project=firefox, IIRC that won't try to install a headless Firefox. But it might require channel: 'firefox' attribute in the firefox entry in playwright.config.js. Or it might only work with Chrome; At least with Chrome I know that it is possible because that's how the CI run is so fast: It does not install any backend.

Playwright test added.

Awesome!

dscho
dscho approved these changes Oct 18, 2024
View reviewed changes
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

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

Excellent work, thank you so much!

@dscho dscho merged commit 50f92d2 into git:gh-pages Oct 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dscho dscho dscho 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.
2 participants
@To1ne @dscho