Make status bar accessible at 400% zoom by hiding items with priority of zero (default)

[j264415]

References

• Closes Status bar at 400% zoom #14793
• Relates to 400% Zoom screen usability enhancements - Updated #14766

Code changes

• new priority argument is added to IStatusBar.IRankItem, which defaults to 0
• when window is narrow (less or equal to 630 pixels, which is a heuristic for 400% zoom) the items with priority of zero get hidden so that the higher priority items remain visible

User-facing changes

Currently with 400% zoom at 1280px the status bar elements overlap each other which hinders usability. Unrequired elements have been removed and the remaining elements no longer overlap at 320px apparent width (1280px/400%).

This means that when a window is below 320px apparent width, the kernel title, document name and the processor icons will be hidden from view to allow space for the simple switch and the line-column number to be fully visible and accessible. If future text elements are added these will also be hidden unless they are given the priority-zoom class.

Backwards-incompatible changes

Extensions which add custom status bar items may want to set custom priority greater than one if these need to be visible at narrow screens.

[jupyterlab-probot]

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

[j264415]

Please could this PR be reviewed. @krassowski

[j264415]

Please could this PR be reviewed. @gabalafou

[gabalafou]

While I think this PR has some great ideas, it also makes me uncomfortable because I feel like it's trying to achieve at the CSS level what should be achieved at the app/JS level.

For example, the StatusBar class provides a registerStatusItem method. This method is used by extensions, including built-in extensions, to add items to the status bar, like so:

// Add the status item to the status bar.
statusBar.registerStatusItem(lineColItem.id, {
  item,
  align: 'right',
  rank: 2,
  isActive: () => !!item.model.editor
});

It seems to me that this is the point at which something like your "priority-zoom" property should be passed.

I also question some of the decisions about what is and isn't priority in this PR. To me the notification icon is a better use of space than the simple-mode toggle—both because the toggle takes up a lot of space and also because the same command can be accessed in two other ways, via the menu and via the command palette; also there are other more obvious cues about when the app is in simple mode or the not, so I'm not sure how much value is added by always showing that state in the status bar.

I also question how effective this approach is because, for example, here's a screenshot on my laptop with the UI zoomed to 400% and it shows that the changes in this PR have done nothing to help because my laptop screen at 400% does not trigger the at-media rule:

I agree that the status bar is too cluttered at narrow width and/or high zoom. But I'm not sure this is the way to solve it. Or maybe this is the right direction, maybe the items in the status bar need to indicate some priority-level, and then if the status bar doesn't have enough width for all of the items, it starts to remove the lower-priority ones. But I think that functionality probably needs to implemented at the level of the StatusBar TypeScript class.

[j264415]

Hi @gabalafou looking at the comment you made above about implementing the changes that you want for the 400% zoom through the app/JS level and not CSS; I have made the relevant code changes required to achieve this.
Please could you review these changes.
Thank you

[j264415]

please update galata snapshot

[j264415]

please update documentation snapshots

[github-actions]

Documentation snapshots updated.

[j264415]

Hi @gabalafou could you please review the changes I have made from your feedback.
Thank you.

[krassowski]

Question from @gabalafou on the call: is there a justification for this specific number?

[j264415]

This number is because this is when the status bar starts to become inaccessible. The comment above has been reviewed by @gabalafou and changed to make the reasoning of this number clearer.

[gabalafou]

Thank you for working on this! I added just a few suggestions to reflect the last changes in the comments and method names

[j264415]

Thank you for working on this! I added just a few suggestions to reflect the last changes in the comments and method names

Thank you for reviewing this !

[gabalafou]

This looks good to go from my perspective. The only check failing is UI Tests / Visual Regression, which may be unrelated to this PR.

If there are no objections, I will merge it in on Monday, December 18.

[krassowski]

This looks good to go from my perspective.

Can you approve it? It seems there is still a an "X" from you. I often sort by approved when merging things before release.

The only check failing is UI Tests / Visual Regression, which may be unrelated to this PR.

I think they are unrelated, but I kicked it

If there are no objections, I will merge it in on Monday, December 18.

I might merge it earlier to fit it in beta.0.

[krassowski]

Thank you @j264415!

[krassowski]

@j264415 I updated the top-level comment's Code Changes and Backwards-incompatible changes sections. Please take note for future PRs that we need to keep them aligned with the content of the PR and they can be brief, as long as the key take-away messages are conveyed :)

@j264415 can you please follow with another pull request adding a note to extension migration guide about the new priority attribute and what it entails for extension developers (see my note in Backwards-incompatible changes section, feel welcome to expand on this with an example).

[j264415]

@j264415 can you please follow with another pull request adding a note to extension migration guide about the new priority attribute and what it entails for extension developers (see my note in Backwards-incompatible changes section, feel welcome to expand on this with an example).

@krassowski All done. This is the PR: #15556