-
Notifications
You must be signed in to change notification settings - Fork 376
feat(CodeEditor): use custom PatternFly monaco theme #11785
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
|
Preview: https://pf-react-pr-11785.surge.sh A11y report: https://pf-react-pr-11785-a11y.surge.sh |
|
For the Syntax colours I am going to create a design issue to look at some code languages and pick out a colour scheme that will work in light/dark mode. I'm fine with the default palette from Monaco for the time being if it isn't blocking anything critical. |
thanks! feel free to edit this branch if the design team makes changes 👍 |
|
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
packages/react-code-editor/src/components/CodeEditor/CodeEditor.tsx
Outdated
Show resolved
Hide resolved
thatblindgeye
left a comment
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.
Overall lgtm, had a nit below but not blocking to get this in (we can always make a change at any other point).
packages/react-code-editor/src/components/CodeEditor/themeTokenMapping.ts
Outdated
Show resolved
Hide resolved
438f37b to
4f80000
Compare
Is there anything at all you would change with the monaco theme in this PR? There were concerns around the OCP YAML editor's lack of contrast (despite it exceeding the 7:1 AAA contrast recommendation 🫤), so we should make sure this theme doesn't stir too many pots |
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.
We'll need to update any hard coded colours and apply some tokens or create new ones. The colours seem to be AA compliant so I think it's good to go.
mcoker
left a comment
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.
🎃
What:
Closes #11506
Closes #11764
Includes the changes from #11575 but now it overwrites every token from the base vs theme
Note there should be some additional CSS in PatternFly to make this look better (so that the margin around the CodeEditor matches the monaco background color), but I'm not sure of the best way to implement it:
I tried looking into using
pf-t--global--background--color--primary--default, but it doesn't provide enough contrast in dark mode. Meanwhile,pf-t--global--background--color--secondary--defaultdoesn't provide enough contrast in light mode.