uui-box: add a property to control the headline variant #521
Conversation
|
Hi there @Digipex, thank you for this contribution! 👍 While we wait for the team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
|
Hi @Digipex thanks for your contribution, well done. And I'm happy to see that this does not enable the developer to visually pick different headline sizes, but just enabling to correct the tag for Accessibility concerns. I've added a comment to emphasize this, so no one in the future thinks that's a mistake. I also did a few minor corrections to the code, to make it work properly with ESLint. I'm unsure when those commits will be picked up by GitHub. But when they are in place I will approve and merge :-) |
|
Thanks for the feedback. Yeah was struggling to figure out the ESLint issue so be good to see what you did. |
…t when the property changes, also use the literal-literal for static strings
There was a problem hiding this comment.
I have changed the following:
- Rename
headerVarianttoheadlineVariantto better reflect what it controls/changes (headline) - Moved call to unsafeStatic() out of render() method and used the literal-literal following recommendation from Lit
- Added a test case to ensure the new property works as intended
- Fixed prettier formatting
- Fixed typings.d.ts to allow story files that references markdown files to pass the
tsc-filescheck
LGTM.
iOvergaard
changed the title
* uui-box: add a property to control header variant * allow to commit /.vscode/settings.json * allow for vscode to store workspace cSpell.words * comment, rendering optimization + disable eslint * fix: tell typescript raw files can be loaded * feat: rename to headlineVariant to better reflect what the property changes * feat: move unsafeStatic call out of render() method and only assign it when the property changes, also use the literal-literal for static strings * chore: prettier formatting * docs: update README for headline-variant * test: update to reflect changed property name * test: add test to ensure headlineVariant works as intended --------- Co-authored-by: Niels Lyngsø <[email protected]> Co-authored-by: Jacob Overgaard <[email protected]>
Description
fixes #447
Types of changes
Motivation and context
How to test?
Story at /docs/layout-box--docs been updated to include new property headerVariant
Change dropdown to a different H tag and inspect element to see component now renders with the H tag selected rather than then always h5
Screenshots (if appropriate)
Checklist