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

UX for LG/LU syntax highlight #776

Closed
hibrenda opened this issue Aug 30, 2019 · 17 comments
Closed

UX for LG/LU syntax highlight #776

hibrenda opened this issue Aug 30, 2019 · 17 comments
Assignees
Labels
Area: Shell P0 Must Fix. Release-blocker R7 Release 7 - December 10th, 2019

Comments

@hibrenda
Copy link
Contributor

need to be consistent for lu/lg all up view and also for inline editor

@cwhitten
Copy link
Member

ping @DesignPolice :)

@DesignPolice
Copy link

got it

@vishwacsena vishwacsena added the Area: LU Belongs to LU feature area label Oct 2, 2019
@cwhitten
Copy link
Member

cwhitten commented Oct 4, 2019

ping @DesignPolice do we have a decision on colors?

@DesignPolice
Copy link

I had made one, but got some different feedback, let me circle back and see if one of the new options can get approved.

@cwhitten cwhitten removed the 4.6 label Oct 7, 2019
@sangwoohaan sangwoohaan removed their assignment Oct 15, 2019
@DesignPolice
Copy link

I worked with Dong before he left town on this, things were looking a bit too orange for me, the cardinal was long standing color, my intention was just to make something that was a traditional VS color styling that used our colors when possible - but open to thoughts.

orange and cardinal are pretty similar hues

Here is the current linked Figma file
https://www.figma.com/file/Ux0vucOvaLL2xH22WpFgV65X/MarcB_Composer-UI?node-id=3270%3A231

It can be a change next week, or CW and Vish can just make the tweaks. - M
Screen Shot 2019-10-16 at 5 26 32 PM

@hibrenda hibrenda added the R7 Release 7 - December 10th, 2019 label Oct 20, 2019
@DesignPolice
Copy link

@cwhitten @boydc2014

This looks like this was backlogged, not a big deal - This was the last we looked at. v13 - My hope was to just get colors to be as familiar as we could get using VS "traditional" colors that were close to our color palettes -

https://www.figma.com/file/Ux0vucOvaLL2xH22WpFgV65X/MarcB_Composer-UI?node-id=3707%3A27305

Screen Shot 2019-11-01 at 2 38 57 PM

@hibrenda hibrenda removed the Area: LU Belongs to LU feature area label Nov 7, 2019
@cwhitten cwhitten added P0 Must Fix. Release-blocker and removed UX Design Need UX Design labels Nov 7, 2019
@compulim compulim changed the title UX for LG/LU syntext highlight UX for LG/LU syntax highlight Nov 19, 2019
@zhixzhan
Copy link
Contributor

Current color theme references vscode's recommend, use not more than 5 color (most-used by other languages) highlighting the document.

The color customization is written in this file:

monaco.editor.defineTheme('lgtheme', {
base: 'vs',
inherit: false,
colors: {},
rules: [
{ token: 'template-name', foreground: '0000FF' },
{ token: 'function-name', foreground: '79571E' },
{ token: 'keywords', foreground: '0000FF' },
{ token: 'comments', foreground: '7A7574' },
{ token: 'number', foreground: '00A32B' },
{ token: 'string', foreground: 'DF2C2C' },
{ token: 'structure-name', foreground: '00B7C3' },
],

preview with color:

image

@zhixzhan zhixzhan self-assigned this Nov 19, 2019
@hibrenda
Copy link
Contributor Author

@DesignPolice need your suggestions

@DesignPolice
Copy link

Thanks @hibrenda and @zhixzhan we will discuss it this morning -

@hibrenda
Copy link
Contributor Author

hibrenda commented Dec 2, 2019

Need discussion for syntax highlighting (template vs func) @DesignPolice @cwhitten
image
image

@DesignPolice
Copy link

Thanks Brenda, I will touchbase with Chris and Vish on this to get some feedback - thanks M

@DesignPolice
Copy link

DesignPolice commented Dec 3, 2019

Attached are the colors that Dong and I looked at - I don't use code editors - so my thought was that we should use Fabric colors that align with the default colors in Visual Studio. There are a couple of options on a few of them.

Making it as consistent as possible. If @cwhitten or @vishwacsena can help me verify that.

Screen Shot 2019-12-02 at 4 34 35 PM

@DesignPolice
Copy link

I talked to @tdurnford and he pulled up the default Visual Studio coloring - they have a variety of themes which would be a nice add for us in the future. Here is their base color on white. So maybe I can just sample colors from this in the Fabric palette - does anyone have issues with that? I don't think it is beautiful, but with only one option I think it should be familiar to start with.

MicrosoftTeams-image (5)

@boydc2014
Copy link
Contributor

I talked to @tdurnford and he pulled up the default Visual Studio coloring - they have a variety of themes which would be a nice add for us in the future. Here is their base color on white. So maybe I can just sample colors from this in the Fabric palette - does anyone have issues with that? I don't think it is beautiful, but with only one option I think it should be familiar to start with.

MicrosoftTeams-image (5)

No problem at all with keyword as blue, and string literal as dark red (right?), it's just we have more stuff, like template-name, and function(template-ref), we also need some color for that

@boydc2014
Copy link
Contributor

This is a good example our colors today
image

@DesignPolice
Copy link

okay. After some frustration, I went through and checked colors on Visual Studio default colors, that hit contrast numbers. These descriptions came from @boydc2014 chatting about the different options.

so looking at the two attachments these are Composer colors, that hit contrast numbers, that are pretty similar to this VS default. There are lots of options built in to the Monaco editor that help with Accessibility

https://github.com/Microsoft/monaco-editor/wiki/Monaco-Editor-Accessibility-Guide

So lets try plugging these colors in and see how v14 looks in reality
Screen Shot 2019-12-03 at 11 39 20 AM
Screen Shot 2019-12-03 at 11 39 03 AM

thanks

@cwhitten
Copy link
Member

cwhitten commented Dec 5, 2019

addressed in #1706

@cwhitten cwhitten closed this as completed Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Shell P0 Must Fix. Release-blocker R7 Release 7 - December 10th, 2019
Projects
None yet
Development

No branches or pull requests

9 participants