Fix top bar visibility not picking up settings overrides (#6833)

[yumyumqing]

• The header part reflects both user preference and default settings overrides.
• Toggle 'Show Header' turns off the automatic adjustment.
• Before this commit behavior:
  • Automatically adjust header visibility to screen size at first
  • If manually check/uncheck the visible box in the settings editor, not automatically adjust header visibility.
  • If manually toggle Show Header in menu->view, not automatically adjust header visibility.
  • If overrides default settings at runtime, it doesn't reflect on the UI.
• After this commit behavior:
  • Automatically adjust header visibility to screen size at first
  • If manually uncheck the adjustToScreen box in the settings editor, not automatically adjust header visibility.
  • If manually toggle Show Header in menu->view, not automatically adjust header visibility.
  • If overrides default settings at runtime, it does reflect on the UI.
• Related issue: Header part not reflecting Jupyter Notebook Top Area default setting #6833

[github-actions]

👈 Launch a Binder on branch yumyumqing/notebook/top-setting-overrides

[yumyumqing]

Here are some expected behaviors in different cases:

No default settings override + no user preference override:

• This is the same as the earlier default behavior.

• Some key variables output:

  [TOP]: visible/composite => true
  [TOP]: visible/user => undefined
  [TOP]: setHidden => false
  [TOP]: adjustToScreen/composite => true
  [TOP]: adjustToScreen/user => undefined
  [TOP]: noAdjust => false

• Behavior:

  • 2 boxes both checked with no Restore to Defaults link.
    

  • adjustToScreen setting overrides the visible setting, with the header showing on large screen and not showing on small screen.
    
    

Default settings override: visible -> false, adjustToScreen -> false + no user preference override:

• Some key variables output:

  [TOP]: visible/composite => false
  [TOP]: visible/user => undefined
  [TOP]: setHidden => true
  [TOP]: adjustToScreen/composite => false
  [TOP]: adjustToScreen/user => undefined
  [TOP]: noAdjust => true

• Behavior:

  • 2 boxes both not checked with no Restore to Defaults link.
    

  • Not automatically adjusting the header visibility, the header is not showing on large or small screen.
    
    

(Some other cases, eliminating the outputs and screenshots for easier read.)

1. Default settings override: adjustToScreen -> false + no user preference override
   • 1 box checked and 1 not checked with no Restore to Defaults link.
   • Not automatically adjusting the header visibility, the header is showing on large and small screen.
2. No default settings override + user preference override: visible -> false, adjustToScreen -> false
   • Both boxes not checked with a Restore to Defaults link.
   • Not automatically adjusting the header visibility, the header is not showing on large or small screen.
3. No default settings override + user preference override: adjustToScreen -> false
   • 1 box checked and 1 box not checked with a Restore to Defaults link.
   • Not automatically adjusting the header visibility, the header is showing on large and small screen.
4. More other cases not listed here.

[yumyumqing]

Hi @jtpio just a friendly reminder on this PR :) Have you looked at it and/or is there anything I need to add? Thanks!

[jtpio]

@yumyumqing sorry for the late reply!

It looks good on Binder and should indeed fix the main usability issue reported in #6833.

Looking at the code and the new setting I was wondering if there was a way to combine the two options into a single one. But maybe it's simpler and more explicit to keep the two options separated.

[yumyumqing]

Hi @jtpio thanks for your response!

If we would like to keep the exact same old variables as before to keep users' current overriding files work, I couldn't think of a better way to merge it in 1 variable.
But if we are willing to break some of the current behavior, I think we can turn TopBarVisibility into a string variable, and then user can input yes for always showing, no for always not showing, automatic for adjusting to size. And we can default to automatic to match the current default behavior.

Do you have any advice on that? Thanks!

[jtpio]

I think we can turn TopBarVisibility into a string variable, and then user can input yes for always showing, no for always not showing, automatic for adjusting to size. And we can default to automatic to match the current default behavior.

That sounds like a good idea. JupyterLab uses enums like the one proposed here for some settings. It's probably fine to "break" the current behavior now since it's a beta, and details like this one are still subject to changes. We can also add an more visible comment in the changelog afterwards to highlight this if needed.

[yumyumqing]

Hi @jtpio I have updated the PR according to our discussion. Could you please help take a look? Thanks a lot!

[jtpio]

Thanks!