Add validation to new settings editor#56176
Conversation
JacksonKearl
changed the title
|
Looks like there's some tests that need updating |
|
Never mind, looks like that's unrelated, despite being a tree issue. |
roblourens
left a comment
roblourens
left a comment
There was a problem hiding this comment.
There are some merge conflicts, some things in settingsTree.ts moved to settingsTreeModels.ts, I can help you fix it up if needed.
| } else if (isArray(setting.type) && setting.type.indexOf('null') > -1 && setting.type.length === 2) { | ||
| if (setting.type.indexOf('number') > -1) { | ||
| element.valueType = 'nullable-integer'; | ||
| } else if (setting.type.indexOf('number') > -1) { |
There was a problem hiding this comment.
And what about types that fall out of this check and end up with no valueType?
| const descriptionPreValue = indent + '// '; | ||
| for (let line of setting.description) { | ||
| // Remove setting link tag | ||
| for (let line of (setting.deprecationMessage ? [setting.deprecationMessage] : setting.description)) { |
There was a problem hiding this comment.
It only writes the deprecation message instead of the description?
There was a problem hiding this comment.
Could make it write both, but the concern from earlier was that it was not totally clear that the item was deprecated. Maybe it would be better with the deprecation message at the top? What do you think?
There was a problem hiding this comment.
I think it should have both. How about just prepending the deprecationMessage in * so it gets rendered in italics? And we can decide whether we want something fancier later.
There was a problem hiding this comment.
As discussed, this is the raw text model so we dont have italic support. In the new editor we use the special deprecation warning element anyways
|
|
|
@roblourens I think this should be ready to go. |
3 participants
Closes #55801