-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Enable prettier formatter in monaco #12640
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
base: sandcastle-react-structure
Are you sure you want to change the base?
Conversation
Thank you for the pull request, @jjspace! ✅ We can confirm we have a CLA on file for you. |
We all have editor preferences, and some user expressed interest in disabling as much as possible. How tricky would it be to add a settings menu to turn features on and off? |
@ggetz functionally it shouldn't be that hard. However it would probably be a lot of effort to pick and choose which of the 150+ options we want to expose and allow changing I would rather we stick mostly to defaults with the few opinionated changes I've already made. Monaco shouldn't really get in the way of people just "typing in code". If we get feedback that it's too much after trying it we can potentially add a single toggle for a "plain text" mode that turns off nearly everything. |
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.
Looks good to me!
guides: { | ||
bracketPairs: "active", | ||
}, | ||
minimap: { size: "fill" }, |
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.
Honestly hate this. 😆 But I'm not going to hold up the PR over it.
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.
haha I was really on the fence about this one. I personally really hate it the "default" way and this is much better to me. 😆 This is probably the most opinionated setting I changed.
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.
Honestly thought about just removing the minimap entirely 🤔
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.
Yeah, personally I hate the minimap entirely.
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.
Well you're in luck 😆 Due to how monaco calculates resizing the minimap seems "expensive" and slow. When I added the panel resizing (#12672) it started flickering pretty bad and lagging behind visually. Easier to just turn it off (in that PR) Oh well 🤷
Yes, I agree that approach it with that kind of fine-toothed comb would be a loosing battle. At the very least, we are already allowing user to toggle dark and light modes, so I don't think it would hurt to have a settings area for that plus maybe one or two "controversial" options. From there, we could make small changes based on user feedback, right? |
Description
prettier
to Sandcastle to format code in monacoIssue number and link
Part of #12566
Testing plan
Format
and make sure it works for JS and HTML with no errorsAuthor checklist
CONTRIBUTORS.md
CHANGES.md
with a short summary of my change