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

Scrollbar size settings are "dendrified" #11732

Closed
per1234 opened this issue Oct 3, 2022 · 4 comments · Fixed by #11883 · May be fixed by EstFoisy/theia#6
Closed

Scrollbar size settings are "dendrified" #11732

per1234 opened this issue Oct 3, 2022 · 4 comments · Fixed by #11883 · May be fixed by EstFoisy/theia#6
Labels
good first issue good first issues for new contributors monaco issues related to monaco

Comments

@per1234
Copy link
Contributor

per1234 commented Oct 3, 2022

Bug Description:

When a non-default value is set for the editor.scrollbar.horizontalScrollbarSize or editor.scrollbar.verticalScrollbarSize settings, the default value is still used for all new editors/windows.

Steps to Reproduce:

  1. Run the browser example.
  2. Select File > New File from the menus.
  3. Hold the Space key until a horizontal scrollbar appears.
  4. Hold the Enter key until a vertical scrollbar appears.
  5. Open the Command Palette.
  6. Execute the "Preferences: Open Settings (JSON)" command.
  7. Set the editor.scrollbar.horizontalScrollbarSize and editor.scrollbar.verticalScrollbarSize settings to 50:
    {
        "editor.scrollbar.horizontalScrollbarSize": 50,
        "editor.scrollbar.verticalScrollbarSize": 50,
    }
  8. Select the previously created editor.
    🙂 The horizontal and vertical scrollbar "troughs" are sized according to the custom setting.
    ❗ There is an unrelated bug that causes the "thumbs" to remain at the default size.
  9. Select File > New File from the menus.
  10. Hold the Space key until a horizontal scrollbar appears.
  11. Hold the Enter key until a vertical scrollbar appears.

🐛 The horizontal and vertical scrollbar "troughs" are sized according to the default setting (14).

Additional Information

  • Operating System: Windows 10
  • Theia Version: d3f6359

Related: #11709

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Oct 4, 2022

The reason those settings are not applied is that those values are currently hard-coded in monaco-editor:

scrollbar: {
...options?.scrollbar,
useShadows: false,
verticalHasArrows: false,
horizontalHasArrows: false,
verticalScrollbarSize: 10,
horizontalScrollbarSize: 10
}

In my earlier PR, I chose not to modify that in case there was a good reason for having those values set that way, but either removing the hard-coded values or switching the spread to the end of the object would allow those preference values to take effect in new editors. Moving the spread to the bottom is probably the best approach.

@colin-grant-work colin-grant-work added monaco issues related to monaco good first issue good first issues for new contributors labels Oct 4, 2022
@ravitejasssihl
Copy link

ravitejasssihl commented Oct 19, 2022

Hey @colin-grant-work can you give me a bit more idea on what changes are you expecting? I am a complete beginner and would like to up the issue.

@colin-grant-work
Copy link
Contributor

@ravitejasssihl, basically, at the moment, we're spreading the user-supplied options, then providing defaults. That means that anywhere the defaults conflict with user options, the defaults win, which isn't desirable. Switching the code here to something like this would allow the user options to win out:

        const combinedOptions = {
            lightbulb: { enabled: true },
            fixedOverflowWidgets: true,
            ...options,
            scrollbar: {
                useShadows: false,
                verticalHasArrows: false,
                horizontalHasArrows: false,
                verticalScrollbarSize: 10,
                horizontalScrollbarSize: 10,
                ...options?.scrollbar,
            }
        } as IStandaloneEditorConstructionOptions;

@per1234
Copy link
Contributor Author

per1234 commented Nov 6, 2022

@svenefftinge I see you were the one who set the scrollbar widths to this nonstandard size they are now locked to: #852

Would you be able to clear up @colin-grant-work's doubt about whether the locking of the setting was done intentionally:

I chose not to modify that in case there was a good reason for having those values set that way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue good first issues for new contributors monaco issues related to monaco
Projects
None yet
3 participants