Skip to content
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

feat(vwc-helper-message): commonalize text helper component #575

Merged
merged 15 commits into from
Jan 15, 2021

Conversation

gullerya
Copy link
Contributor

textarea, textfield, select and file-picker - are all using the same logic to show a helper message which can turn to be an error message with error icon on demand.

This component make them all using the same thing, reduces amount of duplicate code, dependencies on some internal stuff of MWC and also available as a stand-alone functionality for the consumer applications.

@gullerya gullerya added the enhancement New feature or request label Jan 14, 2021
@gullerya gullerya requested review from YonatanKra and yinonov and removed request for YonatanKra January 14, 2021 10:19
@github-actions
Copy link

🚀

Latest successful build of the PR deployed here.

🚀

@github-actions
Copy link

github-actions bot commented Jan 14, 2021

File Coverage
All files 80%
common/context/src/vvd-context.ts 96%
common/fonts/src/vvd-fonts.ts 83%
common/foundation/src/form-association/associate-with-form.ts 90%
common/foundation/src/form-association/common.ts 90%
common/foundation/src/form-association/submit-on-enter-key.ts 80%
common/scheme/src/os-sync.utils.ts 58%
common/scheme/src/vvd-scheme-style-tag-handler.ts 79%
common/scheme/src/vvd-scheme.ts 82%
components/audio/src/vwc-audio.ts 60%
components/button/src/vwc-button.ts 79%
components/carousel/src/vwc-carousel.ts 71%
components/drawer/src/vwc-drawer.ts 42%
components/fab/src/vwc-fab.ts 63%
components/file-picker/src/vwc-file-picker.ts 67%
components/icon-button-toggle/src/vwc-icon-button-toggle.ts 67%
components/icon-button/src/vwc-icon-button.ts 88%
components/icon/src/vwc-icon.js 92%
components/list/src/vwc-list-expansion-panel.ts 73%
components/list/src/vwc-list-item.ts 80%
components/media-controller/src/vwc-media-controller.ts 84%
components/select/src/vwc-select.ts 90%
components/slider/src/vwc-slider.ts 88%
components/textarea/src/vwc-textarea.ts 81%
components/textfield/src/vwc-textfield.ts 90%
components/theme-switch/src/vwc-theme-switch.ts 88%

Minimum allowed coverage is 75%

Generated by 🐒 cobertura-action against 64374cf

@yinonov
Copy link
Contributor

yinonov commented Jan 15, 2021

I'm skipping review of yaml files

@sonarcloud
Copy link

sonarcloud bot commented Jan 15, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@@ -36,7 +36,7 @@ jobs:
path: |
${{ steps.yarn-cache-dir-path.outputs.dir }}
**/node_modules
key: vivid-cache-yarn-${{ hashFiles('**/yarn.lock') }}
key: vivid-cache-yarn-${{ hashFiles('**/package.json') }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this likely to change in the future? can it be an environment variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think so - this is just about to clean our cache each time anything related to node_modules changes

protected render(): TemplateResult {
return html`
<vwc-icon class="helper-icon" type="info-negative" size="small"></vwc-icon>
<span class="helper-text"><slot></slot></span>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideas for a11y, aria, wai-aria?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP, I'll think about and we'll do some improvements round (opening a ticket for that)

@gullerya gullerya merged commit 5c152d5 into master Jan 15, 2021
@gullerya gullerya deleted the tech-commonalize-text-helper-component branch January 15, 2021 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants