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

400% zoom low vision #15855

Open
wants to merge 291 commits into
base: main
Choose a base branch
from
Open

Conversation

g547315
Copy link
Contributor

@g547315 g547315 commented Feb 22, 2024

References

#14626
#14766

#10004 This PR covers Full low Vision Support (400% Zoom) (Mobile/Tablet-Responsive Support).
It resolves most of the failing screenshots that where present in #14766. The approach used was to isolate each code change in the previous PR to see what/ If the code changes are breaking screenshots.

Isolated Code changes and their effects on Screenshots

Code changes

Changed padding and spacing to use min CSS function alongside vh (viewport-height) and vw(viewport-width) to reduce proportional spacing at high zoom and increase useful screen real estate usage.

User-facing changes

Currently with the 400% zoom at 1280px the help main menu bar does not appear and within the file-browser tab bar only half of one 'dir-listing' shows up.

The changes I have made now allows for the help dropdown to be shown in the main menu. I found that when trying to check the changes the sidebar 'Extension Manger' disappears from view, so this was also changes so that users that needs this implemented would be able to have access to it without zooming back out.
Furthermore, within the file-browser tab, users would now be able to see least 2 notebook directory listed below than before.

Backwards-incompatible changes

j264415 and others added 30 commits September 18, 2023 16:02
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@gabalafou
Copy link
Contributor

gabalafou commented Mar 15, 2024

The only way I see forward on this PR is breaking it up into several individual PRs where we change one and only one CSS style rule at a time.

This may sound like a contradiction of something I said in a previous meeting about not opening, closing, and duplicating PRs. However, this PR is a unique situation. We're really struggling to figure out what's causing some of the snapshots to fail, so I think unfortunately, the best path forward is to break these changes up and handle them one at a time.

@gabalafou
Copy link
Contributor

gabalafou commented Mar 15, 2024

One other thing. I think we need to establish a point of view about when zoom-related changes should start to kick in. For example the selector .jp-FileBrowser-toolbar.jp-Toolbar has a margin-top rule that looks like min(5vh, 8px), which I believe starts to kick in at viewport height below 160px (because 5vh * 160 = 8), whereas the selector .jp-BreadCrumbs has min(1.2vh, 8px), which corresponds to viewport height circa 666px. I don't know why one of these should start to kick in for viewport heights below 160px while the other kicks in at viewport heights below 666px.

@g547315
Copy link
Contributor Author

g547315 commented Mar 20, 2024

The only way I see forward on this PR is breaking it up into several individual PRs where we change one and only one CSS style rule at a time.

This may sound like a contradiction of something I said in a previous meeting about not opening, closing, and duplicating PRs. However, this PR is a unique situation. We're really struggling to figure out what's causing some of the snapshots to fail, so I think unfortunately, the best path forward is to break these changes up and handle them one at a time.

Isolated Code changes and their effects on Screenshots

@gabalafou
Copy link
Contributor

Right. But those PRs are each against a fork of jupyterlab rather than jupyterlab/jupyterlab.

I was thinking that we should open the isolated changes as PRs against the jupyterlab/jupyterlab repo, and I'm hoping that handling them one at a time will make it easier to review them and get them in.

As far as I can tell, there's no reason that all the CSS changes in this PR need to be merged together.

@gabalafou
Copy link
Contributor

I'd be happy to do the work to break up the PR into separate PRs, but I don't know how to do that without using my own Git commits, which would change who the changes get attributed to, so I don't want to do that without asking your permission first.

@g547315
Copy link
Contributor Author

g547315 commented Mar 21, 2024

Happy for you to break up the PR into separate PRs

@krassowski
Copy link
Member

Should we close this PR in favour of #14766 or the other way? Why are there so many PRs?

The number of PRs opened and number of spurious commits has really drained my maintenance energy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants