Skip to content
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.

Sign up for GitHub

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

Jump to bottom

test(WebTerminal): add default state AVT check #5171

Merged
merged 18 commits into from
May 22, 2024
Merged

test(WebTerminal): add default state AVT check #5171

merged 18 commits into from
May 22, 2024

Conversation

anamikaanu96
Copy link
Contributor

Closes #5090

Created e2e/components/WebTerminal/WebTerminal-test.avt.e2e.js

What did you change? e2e/components/WebTerminal/WebTerminal-test.avt.e2e.js

How did you test and verify your work? yarn avt

@anamikaanu96 anamikaanu96 requested a review from a team as a code owner May 14, 2024 08:26
@anamikaanu96 anamikaanu96 requested review from elycheea and IgnacioBecerra and removed request for a team May 14, 2024 08:26
@netlify Netlify
Copy link

netlify bot commented May 14, 2024 •
edited

✅ Deploy Preview for carbon-for-ibm-products ready!

Name Link
🔨 Latest commit 23c82f8
🔍 Latest deploy log https://app.netlify.com/sites/carbon-for-ibm-products/deploys/664e14208a7b1e0008237dac
😎 Deploy Preview https://deploy-preview-5171--carbon-for-ibm-products.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

elycheea
elycheea requested changes May 14, 2024
View reviewed changes
Copy link
Contributor

@elycheea elycheea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is another case where we need to click on the web terminal in the global header in order to test this one properly.

There’s a few examples this in recent AVT PRs from Anna, Afsal, and Nandan.

#5127 (comment)

elycheea
elycheea reviewed May 15, 2024
View reviewed changes
cspell.json Outdated Show resolved Hide resolved
elycheea
elycheea reviewed May 20, 2024
View reviewed changes
Copy link
Contributor

@elycheea elycheea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more set of small changes. I think we should set a default or fallback for the aria label rather than requiring it. This will minimize impact to anyone using this component.

Should also reduce the repeated labels in stories and tests if you set it too. 🙂

cspell.json Outdated Show resolved Hide resolved
packages/ibm-products/src/components/WebTerminal/WebTerminal.tsx Outdated Show resolved Hide resolved
elycheea
elycheea previously approved these changes May 21, 2024
View reviewed changes
makafsal
makafsal previously approved these changes May 22, 2024
View reviewed changes
@elycheea elycheea added this pull request to the merge queue May 22, 2024
@matthewgallo matthewgallo removed this pull request from the merge queue due to a manual request May 22, 2024
annawen1
annawen1 requested changes May 22, 2024
View reviewed changes
Copy link
Member

@annawen1 annawen1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one change with the prefix!

annawen1 and others added 2 commits May 22, 2024 09:34
@annawen1
Merge branch 'main' into webTerminalAVT
9e95027
@anamikaanu96 @annawen1
test(WebTerminal): review change
33ebc2e 
Co-authored-by: Anna Wen <54281166+annawen1@users.noreply.github.com>
@anamikaanu96 anamikaanu96 dismissed stale reviews from makafsal and elycheea via 33ebc2e May 22, 2024 15:47
@annawen1 annawen1 enabled auto-merge May 22, 2024 19:13
@annawen1 annawen1 added this pull request to the merge queue May 22, 2024
Merged via the queue into carbon-design-system:main with commit e9441f5 May 22, 2024
17 checks passed
paul-balchin-ibm pushed a commit to paul-balchin-ibm/ibm-products that referenced this pull request May 23, 2024
@anamikaanu96 @annawen1 @paul-balchin-ibm
test(WebTerminal): add default state AVT check (carbon-design-system#…
463c177 
…5171)

* test(WebTerminal): add default state AVT check

* test(WebTerminal): review changes

* test(WebTerminal): review change

* test(WebTerminal): review change

* test(webterminal): review changes

* test(WebTerminal): review changes

* test(WebTerminal): resolve test issue

* test(WebTerminal): review changes

* test(WebTerminal): review change

Co-authored-by: Anna Wen <54281166+annawen1@users.noreply.github.com>

---------

Co-authored-by: Anna Wen <54281166+annawen1@users.noreply.github.com>
@anamikaanu96 anamikaanu96 deleted the webTerminalAVT branch May 24, 2024 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewgallo matthewgallo matthewgallo approved these changes

@annawen1 annawen1 annawen1 approved these changes

@elycheea elycheea elycheea left review comments

@makafsal makafsal makafsal left review comments

@IgnacioBecerra IgnacioBecerra Awaiting requested review from IgnacioBecerra IgnacioBecerra is a code owner automatically assigned from carbon-design-system/carbon-for-ibm-products-reviewers

Assignees
No one assigned
Labels
status: one more review 👀
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add default state AVT check for WebTerminal
5 participants
@anamikaanu96 @elycheea @makafsal @matthewgallo @annawen1