feat(split-button): make downloadable and linkable #11520
Conversation
github-actions
bot
added
the
bug
josercarcamo
added
the
pr ready for visual snapshots
josercarcamo
changed the title
There was a problem hiding this comment.
🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗
🔗⏯️🔗🔗🔗⏯️🔗🔗⏯️⏯️🔗🔗⏯️⏯️⏯️🔗🔗⏯️⏯️⏯️🔗⏯️⏯️⏯️⏯️🔗⏯️🔗
🔗⏯️⏯️🔗🔗⏯️🔗⏯️🔗🔗⏯️🔗🔗⏯️🔗🔗⏯️🔗🔗🔗🔗⏯️🔗🔗🔗🔗⏯️🔗
🔗⏯️🔗⏯️🔗⏯️🔗⏯️🔗🔗⏯️🔗🔗⏯️🔗🔗⏯️🔗🔗🔗🔗⏯️⏯️⏯️🔗🔗⏯️🔗
🔗⏯️🔗🔗⏯️⏯️🔗⏯️🔗🔗⏯️🔗🔗⏯️🔗🔗⏯️🔗🔗🔗🔗⏯️🔗🔗🔗🔗🔗🔗
🔗⏯️🔗🔗🔗⏯️🔗🔗⏯️⏯️🔗🔗⏯️⏯️⏯️🔗🔗⏯️⏯️⏯️🔗⏯️⏯️⏯️⏯️🔗⏯️🔗
🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗🔗
Once comments are addressed, this should be good to merge!
| @@ -21,6 +21,22 @@ describe("calcite-split-button", () => { | |||
| propertyName: "placement", | |||
| defaultValue: "bottom-end", | |||
| }, | |||
| { | |||
There was a problem hiding this comment.
Awesome! Can you also add reflects tests for these new props?
| @@ -287,3 +287,35 @@ export const appearanceAndKindCombinations_TestOnly = (): string => html` | |||
| `; | |||
|
|
|||
| export const loadingAndDisabled_TestOnly = (): string => html`<calcite-button loading disabled>Test</calcite-button>`; | |||
|
|
|||
| export const primaryAsALink_TestOnly = (): string => | |||
There was a problem hiding this comment.
Can you remove the _TestOnly suffixes? These were required at some point, but aren't anymore. We'll be updating story names with #9085, but omitting them now avoids the screenshot test diff the rename will produce.
| const page = await newE2EPage(); | ||
| await page.setContent(`<calcite-split-button href="/">Continue</calcite-split-button>`); | ||
|
|
||
| const elementAsLink = await page.find("calcite-split-button >>> a"); |
There was a problem hiding this comment.
Suggestion: Since this relies on calcite-button’s internal implementation, the test could be simplified to assert that the internal button receives the expected props. This also avoids coupling to its implementation details.
|
Looking at the title and implementation, I think this could use the Also, minor suggestion for consistency: |
|
@ashetland @SkyeSeitz Can you take a look at the new screenshots? This is pretty similar to button, so there shouldn't be any surprises. |
|
Looks good to me! Super nitpick: should the new stories be at medium scale like the rest of them? Truly hesitated to even ask this one 🤣. |
josercarcamo
changed the title
github-actions
bot
added
the
enhancement
josercarcamo
added
pr ready for visual snapshots
josercarcamo
added
pr ready for visual snapshots
Related Issue: #9126
Summary
Implemented ability to make the primary part of a split button into a downloadable link or an href link.