Skip to content

Add support for multiple rulers with different colors#88453

Merged
alexdima merged 7 commits intomicrosoft:masterfrom
mallardduck:color_rulers
Feb 4, 2020
Merged

Add support for multiple rulers with different colors#88453
alexdima merged 7 commits intomicrosoft:masterfrom
mallardduck:color_rulers

Conversation

@mallardduck
Copy link
Contributor

@mallardduck mallardduck commented Jan 10, 2020

This change set:

  • adds an interface for Ruler Color Options,
  • adds a union type of that interface and number,
  • adjust the rulers type hint to use said union type,
  • adds a mechanism to apply boxShadow to fastDomNode, and
  • updates the way rulers are rendered to support individual colors.

I've also ensured that descriptions are provided for settings.json. So now, all nodes will provide explanations.

There may be other ways to implement adjusting the boxShadow - certainly open for input if adding the setBoxShadow isn't as preferred.


This PR fixes #54312 by allowing ruler colors to be defined in settings.json.

This change set:
adds an interface for Ruler Color Options,
adds a union type of that interface and number,
adjust the rulers type hint to use said union type,
adds a mechanism to apply boxshadow to fastDomNode, and
updates the way rulers are rendered to support individual colors.

I've also ensured that descriptions are provided for settings.json.
So now, all nodes will provide explanations.
@msftclas
Copy link

msftclas commented Jan 10, 2020

CLA assistant check
All CLA requirements met.

@methodbox
Copy link

@mallardduck
Looks like the first commit rejected. Take a look?

Thanks for doing the PR. I’m excited to use this feature!

@methodbox
Copy link

Looks like the merge failed due to a problem with the actual test, but not sure. Line 11 just says undefined.

Run .\scripts\test-integration.bat --tfs "Integration Tests"
Active code page: 65001
"Running integration tests out of sources."

undefined

  Search-integration

    √ Text: GameOfLife (51ms)

    √ Text: GameOfLife (RegExp) (57ms)

    √ Text: GameOfLife (unicode escape sequences)

@mallardduck
Copy link
Contributor Author

@methodbox I think that's not the cause of the failure. If you check, the same test for different commits on this PR that message was being thrown in all of them without failure.

@mallardduck
Copy link
Contributor Author

@methodbox, yeah looks fine now.

Just need someone from Microsoft to review, give feedback and/or accept it.

@mallardduck mallardduck requested a review from alexdima January 18, 2020 23:57
@mallardduck
Copy link
Contributor Author

@connor4312 @alexdima - Hey there, I know that I read that multiple ruler colors was in a backlog for a later release point. However I'm wondering if we can get this merged in sooner than later, since I have provided a working implementation?

@alexdima alexdima added this to the February 2020 milestone Feb 4, 2020
@alexdima
Copy link
Member

alexdima commented Feb 4, 2020

Thank you!

@alexdima alexdima merged commit 42582a5 into microsoft:master Feb 4, 2020
@methodbox
Copy link

@alexdima

Any idea when this will make it into distributed builds?

@alexdima
Copy link
Member

alexdima commented Feb 6, 2020

@methodbox This will make it to insiders in the next couple days (insiders is now frozen until stable is shipped), and will come to stable with the February stable release ~4weeks from nwo

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support multiple rulers with different colors

4 participants