-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add new section for UI Settings #3445
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
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typographical: The property markdownBlockLineheight should likely be renamed to markdownBlockLineHeight to adhere to camelCase conventions for consistency with the rest of the file.
This comment was generated because it violated a code review rule: mrule_l9qHIfrVR9RDrBkJ.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
|
Sorry to be late to the discussion here, but I wonder if this really needs to be a new setting in a new section. Should we just make it a little less crammed? |
Thanks for your response. Based on my personal impression, there doesn't seem to be a suitable group in the existing categories for purely UI-related configuration items, as most others are functionally oriented. Therefore, I've created a new one. Which configurations do you think would fit best here? I can make some adjustments accordingly. |
|
Do you think we need a setting for this, or should we just adjust the line height? The current UI looks too tight to me too. |
Actually, there was a quick discussion about this issue before, #3438 I think, it might be be better option, to support full consideration of different users' needs |
Slider just feels like too much for this... I guess gmail has options like compact / default / comfortable? I think I'd just want to have more of an opinion on where we're headed with the visual configuration options before adding this slider. Do any other editors have controls like this? |
This seems to boil down to two issues: one is whether it's necessary to use a new group to manage UI-related configurations, and the other is whether it's necessary to use a slider to adjust row height. Regarding the first question, my starting point is that these configurations might only pertain to the UI rather than functionality, and it seems the new grouping is also appropriate. As for the second issue as you mentioned, the slider is indeed a bit heavier, and most users probably don't need such fine adjustments I think. In comparison, the preset options seem like an excellent choice such as compact, default, comfortable, etc. (Then what the corresponding setting should be for different options? I think maybe compact with 1.25, default 1.5 and comfortable with 1.75) How do you think about these two parts? |
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Any idea about this, guys? |
|
@KrabsWong I think those defaults are reasonable. If lots of people want choice, keep the slider implementation in a stash :-) I guess a numeric input with up / down arrows on the right hand side, to go up by tenths is a decent compromise (assuming shadn has the element to easily use) UI peeps have any opinions? |
The step increment of the slider can also be adjusted, currently it is set to a smaller value. |
|
Hey @KrabsWong, thanks for your contribution! I think the best way to get this PR unstuck is to figure out if the new setting works or if we should just increase the line height for now. Ideally, we'd have more than one UI setting so we can add a new section for them and actually use this new section properly. Let me hear your thoughts about this. |
My initial thought was to simply adjust the line height, which would be a minor change. However, I also received feedback that some users prefer a smaller line height to fit more content on one screen. #3438 |
|
To the question of what else might go into a UI settings section - I think this is a reasonable ask to allow people to enable/disable the code actions: #4214 |
|
stale |
Related GitHub Issue
Closes: #3447
Description
Test Procedure
Type of Change
srcor test files.Pre-Submission Checklist
npm run lint).console.log) has been removed.npm test).mainbranch.npm run changesetif this PR includes user-facing changes or dependency updates.Screenshots / Videos
Current View

Change

line-heightto 1.75 (range from 1.25 to 2)Setting View

Documentation Updates
Add new feature for user to setting UI configurations, currently, there is only one setting to change the
line-heightof content.Additional Notes