Skip to content

Address feedback in #101467#105880

Merged
alexdima merged 2 commits intomicrosoft:masterfrom
IllusionMH:feedback-for-101467
Sep 2, 2020
Merged

Address feedback in #101467#105880
alexdima merged 2 commits intomicrosoft:masterfrom
IllusionMH:feedback-for-101467

Conversation

@IllusionMH
Copy link
Copy Markdown
Contributor

This PR addresses feedback in #101467 (comment)

Couple of notes:

  1. enum now only used to provide autocompletion suggestions and renamed accordingly.
  2. Apparently errorMessage will be shown only from first item in anyOf
  3. type: 'number' should be first otherwise number validation Value is above the maximum of 1000. won't be shown for numbers, however it can be moved lower to have same error message for 1001 and "10001"

/cc @alexdima @jrieken

@alexdima
Copy link
Copy Markdown
Member

alexdima commented Sep 2, 2020

Thank you!

@alexdima alexdima added this to the August 2020 milestone Sep 2, 2020
@alexdima alexdima merged commit 95e7497 into microsoft:master Sep 2, 2020
@IllusionMH IllusionMH deleted the feedback-for-101467 branch September 2, 2020 21:44
@IllusionMH IllusionMH restored the feedback-for-101467 branch September 2, 2020 21:59
@IllusionMH
Copy link
Copy Markdown
Contributor Author

@alexdima I've remembered why I haven't used clampedInt - font-weight: 10000 will be totally ignored by browser therefore my initial check was min <= val <= max to match browser behavior. Demo

I think it's better to match browser behavior and revert code back to min <= val <= max probably add explicit isNaN check if needed.

@alexdima
Copy link
Copy Markdown
Member

alexdima commented Sep 2, 2020

I agree that this is the behaviour of CSS, but these are editor settings, the behaviour of out-of-range values doesn't need to conform to the CSS specification. I think it is OK if we are consistent with our other settings, so we clamp small/large values. IIRC we do the same for fontSize and other editor settings.

@IllusionMH IllusionMH deleted the feedback-for-101467 branch September 2, 2020 22:34
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 17, 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.

3 participants