feat(popover) - update popover component for SHINE - #2030
Conversation
🦋 Changeset detectedLatest commit: b1a4589 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for stacks-svelte ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
giamir
left a comment
giamir
left a comment
There was a problem hiding this comment.
Fantastic work @ttaylor-stack. Love to see a lot of code being removed as part of this PR. I have only left minor comments that you should be able to address swiftly. Feel free to reach out if something I wrote does not make sense. Thank you! ❤️
| #### Popover | ||
| - `s-popover--arrow` removed | ||
|
|
||
| ##### Svelte component |
There was a problem hiding this comment.
I don't think this is relevant for consumers, since the Svelte Popover public API did not change therefore I would recommend to remove it.
| ### Components | ||
|
|
||
| #### Popover | ||
| - `s-popover--arrow` removed |
There was a problem hiding this comment.
Rather than limit ourselves to write what changed I would recommend to add here what the consumers are expected to do. After all this is not a changelog but a migration guide doc. 🙂
So in this case it could be something like.
The new popovers no longer include an arrow element. The `s-popover--arrow` css class has been removed, and any markup using it (e.g. `<div class="s-popover--arrow"></div>`) should be deleted from the codebases as part of the migration.
3 participants
Ticket: https://stackoverflow.atlassian.net/browse/SPARK-74
Update to popover component, removed arrow from component in both svelte and classic