Move theming support from Beta to Main #44
Conversation
# [1.2.0-beta.1](v1.1.6...v1.2.0-beta.1) (2024-07-09) ### Bug Fixes * **hover:** corrects minor visual jump in close button on hover ([7e1a45d](7e1a45d)) ### Features * refactor color token structure with tier 3 tokens [#37](#37) ([4c3e3ce](4c3e3ce)) ### Performance Improvements * replace hard coded values with tokens ([f31eb13](f31eb13))
# [1.2.0-beta.2](v1.2.0-beta.1...v1.2.0-beta.2) (2024-07-20) ### Features * **version:** implement custom versioned components ([f0c717c](f0c717c)) ### Performance Improvements * **icon:** update to use dynamic named icon ([22939fd](22939fd))
Reviewer's Guide by SourceryThis pull request moves theming support from Beta to Main for the auro-toast component. The changes include significant refactoring of the component's structure, styling, and functionality to improve theming capabilities and align with design system standards. Key changes include updating icon handling, implementing custom versioned components, refactoring color token structure, and improving accessibility. File-Level Changes
Tips
|
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @jordanjones243 - I've reviewed your changes - here's some feedback:
Overall Comments:
- The changes to improve theming support and component flexibility look good. However, the reduction of test coverage thresholds to 0% is concerning. What are your plans to improve test coverage?
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 4 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| <auro-toast id="2" visible> Hello I am a toast! </auro-toast> | ||
| `); | ||
|
|
||
| const closeButton = el.shadowRoot.querySelector('[part="close-button"]'); |
There was a problem hiding this comment.
suggestion (testing): Update test to check for correct close button selector
The test has been updated to use the new selector for the close button. It's good to see this change, but we should also add an assertion to ensure the button exists before clicking it.
| const closeButton = el.shadowRoot.querySelector('[part="close-button"]'); | |
| const closeButton = el.shadowRoot.querySelector('[part="close-button"]'); | |
| if (!closeButton) { | |
| throw new Error('Close button not found'); | |
| } |
| await aTimeout(5500); | ||
| const root = el.shadowRoot; | ||
|
|
||
| expect(toastContainer.classList.contains('hidden')).to.be.true; | ||
| await aTimeout(310); | ||
| expect(el.visible).to.be.false; | ||
| }).timeout(5400); | ||
| expect(el.visible).to.be.false; |
There was a problem hiding this comment.
suggestion (testing): Improve test for auto-hiding toast
The test for auto-hiding the toast after five seconds has been simplified. However, it might be beneficial to add an assertion to check if the toast is visible before the timeout, to ensure it's not hidden prematurely.
| await aTimeout(5500); | |
| const root = el.shadowRoot; | |
| expect(toastContainer.classList.contains('hidden')).to.be.true; | |
| await aTimeout(310); | |
| expect(el.visible).to.be.false; | |
| }).timeout(5400); | |
| expect(el.visible).to.be.false; | |
| expect(el.visible).to.be.true; | |
| await aTimeout(5000); | |
| expect(el.visible).to.be.true; | |
| await aTimeout(500); | |
| expect(el.visible).to.be.false; |
| await aTimeout(5050); | ||
| const root = el.shadowRoot; | ||
| const toastContainer = root.querySelector('.toastContainer'); | ||
|
|
||
| expect(toastContainer.classList.contains('hidden')).to.be.false; | ||
| expect(el.visible).to.be.true; | ||
| expect(el.visible).to.be.true; |
There was a problem hiding this comment.
suggestion (testing): Simplify error toast persistence test
The test for error toast not auto-dismissing has been simplified, which is good. Consider adding an assertion at the beginning to ensure the toast is initially visible, providing a more comprehensive test.
| await aTimeout(5050); | |
| const root = el.shadowRoot; | |
| const toastContainer = root.querySelector('.toastContainer'); | |
| expect(toastContainer.classList.contains('hidden')).to.be.false; | |
| expect(el.visible).to.be.true; | |
| expect(el.visible).to.be.true; | |
| const root = el.shadowRoot; | |
| const toastContainer = root.querySelector('.toastContainer'); | |
| expect(el.visible).to.be.true; | |
| expect(toastContainer).to.be.visible; | |
| await aTimeout(5050); | |
| expect(el.visible).to.be.true; | |
| expect(toastContainer).to.be.visible; |
|
|
||
| console.warn(el.shadowRoot.querySelector('.typeIcon')); | ||
| expect(root.querySelector('.typeIcon')).to.not.exist; | ||
| }); | ||
|
|
||
| }); |
There was a problem hiding this comment.
suggestion (testing): Consider adding tests for new functionality
The changes in auro-toast.js introduce new functionality, such as different icons for different variants and the use of auro-button and auro-icon. Consider adding tests to cover these new features and ensure they work as expected.
it('displays correct icon for different variants', async () => {
const variants = ['success', 'error', 'warning', 'information'];
for (const variant of variants) {
const el = await fixture(html`<auro-toast variant="${variant}" visible></auro-toast>`);
const icon = el.shadowRoot.querySelector('auro-icon');
expect(icon).to.exist;
expect(icon.name).to.equal(variant);
}
});
it('respects noIcon attribute', async () => {
const el = await fixture(html`<auro-toast noIcon visible></auro-toast>`);
expect(el.shadowRoot.querySelector('auro-icon')).to.not.exist;
});
BREAKING CHANGE: trigger major release for theming support #37
# [2.0.0-beta.1](v1.2.0-beta.2...v2.0.0-beta.1) (2024-09-26) ### Features * add tag name as attribute when custom registered ([c140d41](c140d41)) ### Performance Improvements * refactor custom component registration config ([ec76cc7](ec76cc7)) * update dependencies ([2831c72](2831c72)) ### BREAKING CHANGES * trigger major release for theming support #37
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Alaska Airlines Pull Request
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Resolves: #37, #41
Summary:
Please summarize the scope of the changes you have submitted, what the intent of the work is and anything that describes the before/after state of the project.
Type of change:
Please delete options that are not relevant.
Checklist:
By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.
Thank you for your submission!
-- Auro Design System Team
Summary by Sourcery
Move theming support for the auro-toast component from beta to main, enabling customization through design tokens. Refactor the component to use dynamic icons and versioned components, and update the styling to leverage design tokens. Enhance documentation with theming examples and update tests to ensure compatibility with the new features.
New Features:
Enhancements:
Documentation:
Tests:
Chores: