-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore(component): stop using the window object in the post-linkarea #4691
chore(component): stop using the window object in the post-linkarea #4691
Conversation
🦋 Changeset detectedLatest commit: e1b8b7f The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
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 |
Related Previews |
@@ -1,30 +1,49 @@ | |||
import { Component, Element, h, Host } from '@stencil/core'; | |||
import { version } from '@root/package.json'; | |||
|
|||
const INTERACTIVE_ELEMENTS = ['a'].join(','); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this constant so that we can refactor the component in the future to also work with buttons (this will be needed for interactive cards). But for now it only accepts anchor elements as it did before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic should also take into account whether the user presses the “Ctrl” key (open in a new tab) or the “Shift” key (open in a new window) when clicking on the element.
if (!this.isInteractive(elementWithDataLink)) { | ||
throw new Error( | ||
`The \`data-link\` attribute must be used on a interactive element inside the \`post-linkarea\` component. Possible elements are: ${INTERACTIVE_ELEMENTS}`, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we should not throw an error if there is at least one interactive element, even though it may not contain the [data-link] attribute. Instead I would log a warning and only throw an error if we really can't find any interactive element. Because then, and only then, the component can not work as expected.
|
No description provided.