-
Notifications
You must be signed in to change notification settings - Fork 379
change: [UIE-8743] - Replaced Button component in DBAAS with Akamai CDS button Web Component #12148
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
Open
tvijay-akamai
wants to merge
13
commits into
linode:develop
Choose a base branch
from
tvijay-akamai:feature/UIE-8743
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
76e0e37
change: [UIE-8743] - Replaced Button component in DBAAS with CDS buttβ¦
tvijay-akamai 8c21f66
change: [UIE-8743] - upgrade web component library version which willβ¦
tvijay-akamai 61cad37
change: [UIE-8743] - resolved merge conflicts
tvijay-akamai 1508b46
change: [UIE-8743] - added test id to some buttons as requested by qa
tvijay-akamai 3afdade
change: [UIE-8743] - added test id to some buttons as requested by qaβ¦
tvijay-akamai 5f59acf
Merge branch 'develop' into feature/UIE-8743
tvijay-akamai 9f32bb7
change: [UIE-8743] - upgraded cds version to fix the button size issue
tvijay-akamai 5ff2e49
change: [UIE-8743] - Adding fixed cypress e2e tests provided and as rβ¦
tvijay-akamai fc7702c
change: [UIE-8743] - fixed cypress e2e test
tvijay-akamai d86d553
Merge branch 'develop' into feature/UIE-8743
tvijay-akamai 3dacb66
Merge branch 'develop' into feature/UIE-8743
cpathipa 6de8a33
change: [UIE-8743] - cypress e2e tests fix from sakshi
tvijay-akamai 7924fd7
change: [UIE-8743] - cypress e2e tests fix from sakshi
tvijay-akamai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
5 changes: 5 additions & 0 deletions
5
packages/manager/.changeset/pr-12148-changed-1746156198791.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@linode/manager": Changed | ||
--- | ||
|
||
Replace the button component under DBAAS with Akamai CDS button web component ([#12148](https://github.com/linode/manager/pull/12148)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 changes: 2 additions & 1 deletion
3
packages/manager/src/features/Databases/DatabaseCreate/DatabaseCreate.style.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@stayal712 and @tvijay-akamai, do you have an explanation for why this workaround is necessary?
In my testing, I'm able to implement a simple (non-CDS) button in a shadow root, set up an event handler, and trigger it by clicking via
cy.click()
. Why doesn't this work with the CDS button? Why aren't we running into any of these same issues with the text field component?I'm willing to resort to hacky workarounds like this if they're necessary, but we should at least understand why. This is a lot of work to click on a button, and the fact that this is only an issue with the CDS button makes it sound like a bug. I want to rule that out, and I especially want to avoid implementing more hacks like this as we introduce more CDS components.
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 am going to try to offer a potential fix, without confidence it'll work. Bear in mind, my knowledge of web components is limited, however I'd still like to explore this before moving forward with the workaround proposed, which seem to be pointing at a potential limitation of the component.
https://git.source.akamai.com/projects/FEE/repos/cds-web-components/browse/packages/cds-web-components/src/components/button/button.element.ts
Custom elements sometimes donβt work well with Reactβs synthetic event system.
onClick
in React wonβt fire unless the component dispatches a standard DOM event from the host. This may be the issue we've been noticing with Cypress.Currently, this listens to @click internally on the native button, but we don't re-emit the event from the custom element. Which could perhaps be done this way:
https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/dispatchEvent
Again, not sure this will fix it, but before approving I'd like to explore this because it would also have important implications for other Web Components making their way to Cloud Manager. It could even be expanded further from what I could read, but not sure how helpful that would be.
Uh oh!
There was an error while loading. Please reload this page.
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.
@abailly-akamai
Thank you for your suggestion. Tarun has started looking into your suggestion and will also explore alternative solutions with QA team to find the best way out to access shadow dom elements in cypress, in the next release. Meanwhile, request you to move forward with this PR for the current release.
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.
Approved. It would be greatly appreciated if the next PR for a web component takes in consideration all the problems raised in this one, and the compatibility with our codebase not to be an afterthought. Merging this as-is gives us no guaranty there will be a follow up. The branch cut is in a week by the way.