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

mgt.dev - changed monaco editor to light theme #350

Merged
merged 12 commits into from
Apr 6, 2020

Conversation

nmetulev
Copy link
Contributor

PR Type

  • Other... Please describe:

Description of the changes

This PR updates the monaco editor used in mgt.dev to use the light theme. By request from: microsoftgraph/microsoft-graph-explorer-v4#452

It also increase the debounce time to 1s (from 0.5s) as I've found the 0.5s to be too fast when editing samples.

image

@nmetulev nmetulev added this to In progress in 1.2 Mar 27, 2020
@nmetulev nmetulev removed this from In progress in 1.2 Mar 27, 2020
@shweaver-MSFT
Copy link
Contributor

This is a bit nit-picky, but...
The top border currently shows at all times. It is really only needed when showing beneath the preview window. The one pixel shift looks slightly off when rendering side-by-side 😁

image

@nmetulev
Copy link
Contributor Author

Good catch, I agree. Fixed

@shweaver-MSFT
Copy link
Contributor

It's doing some weird positioning when I resize the bounds. See if you can repro by playing with the grid splitters a bit.

image

image

@nmetulev
Copy link
Contributor Author

Looks like that was a bug from before - can you check if the latest commit fixes that issue for you?

@shweaver-MSFT
Copy link
Contributor

It's still happening for me with the latest commits. Let me know if you want me to fiddle with it.

Copy link
Contributor

@beth-panx beth-panx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't repro the white space... that looks interesting though. Some small picks commented.

.storybook/addons/codeEditorAddon/editor.js Show resolved Hide resolved
.storybook/addons/codeEditorAddon/editor.js Outdated Show resolved Hide resolved
@shweaver-MSFT
Copy link
Contributor

It's odd that I'm seeing the stale issue and @beth-panx is not.

@vogtn, can you try this out and see if the whitespace issue repros for you?

@vogtn
Copy link
Contributor

vogtn commented Mar 31, 2020

It's odd that I'm seeing the stale issue and @beth-panx is not.

@vogtn, can you try this out and see if the whitespace issue repros for you?

Sorry @shweaver-MSFT I can't seem to repro on my machines/screens :(

@shweaver-MSFT
Copy link
Contributor

@vogtn, I'm glad you can't repro it lol. I think I'm an outlier. I'll try cloning a fresh instance and see if it helps. But don't hold the PR back on my account.

@nmetulev
Copy link
Contributor Author

nmetulev commented Apr 6, 2020

@shweaver-MSFT, have you been able to repro the issue on a fresh clone?

@shweaver-MSFT
Copy link
Contributor

shweaver-MSFT commented Apr 6, 2020

@nmetulev, I still see the issue in Edge, but not in Chrome. If you don't see the issue in your Edge, than I assume it's an issue with my version of Edge. I can't get localhost to work at all in Edge dev.

Copy link
Contributor

@shweaver-MSFT shweaver-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in Chrome.

@shweaver-MSFT
Copy link
Contributor

@nmetulev The latest fixed everything for me :) Good to go

@shweaver-MSFT shweaver-MSFT merged commit 8ec5545 into master Apr 6, 2020
@shweaver-MSFT shweaver-MSFT deleted the mgt-monaco-light branch April 6, 2020 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants