-
Notifications
You must be signed in to change notification settings - Fork 107
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
fix(viewer): filter out invalid dynamic taglist options #315
Conversation
2b3621a
to
468581a
Compare
const schema = '[{"label": "dollar", "value": "$"}]'; | ||
|
||
const description = <div> | ||
Define which input property to populate the values from. The property must follow this schema: <code>{schema}</code>. |
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.
Should we maybe add the description as mentioned in the issue (#305)? So having "The property must follow ..." as another paragraph and add some indentation to the code example.
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.
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.
I like this one much more 👍
Ideally, this should be done by the core (description) styles, yeah, to align it across our tools.
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 could also split this PR into two parts (bug fix, description updates) and, make progress with the bug fix faster + deliver the style + description updates in another PR.
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.
I will make this PR just for the fix, then add a draft PR with the styling changes (to be merged when this is added to the core)
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.
All done: follow up is #316 (already mentioned in the issue)
468581a
to
3ca17b6
Compare
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.
👍 ✅
Related to #303
For the properties panel hint: #305 is for adding a more sophisticated solution, but for now we decided to improve the existing description: