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.

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

Add Test Selector for Checkbox #2302

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AnirudhMani-okta
Copy link
Contributor

@AnirudhMani-okta AnirudhMani-okta commented Jul 25, 2024

OKTA-754187

Summary

Testing & Screenshots

  • I have confirmed this change with my designer and the Odyssey Design Team.

@AnirudhMani-okta AnirudhMani-okta requested a review from a team as a code owner July 25, 2024 22:52
import { Typography } from "./Typography";

export const CheckboxTestSelectors = {
// feature: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

leaving as comment because we will eventually convert this as either a label or a different selector as discussed with @KevinGhadyani-Okta

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we commented this as:

Suggested change
// feature: {
// label: ["hint"]

That'll make it easier in the future because the API won't match the feature API.

@@ -261,6 +262,17 @@ export const Hint: StoryObj<typeof Checkbox> = {
},
play: async ({ canvasElement, step }) => {
await checkTheBox({ canvasElement, step })("Checkbox Hint");
// await step("has visible hint", async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently fails because the hint is included in the label, which it should not
Screenshot 2024-07-25 at 3 55 42 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

@bryancunningham-okta We'll want this noted in a ticket.

For some reason, the hint is setup as part of the label and not the description in Checkbox.

Copy link
Contributor

Choose a reason for hiding this comment

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

@AnirudhMani-okta Can you put a comment above it saying "This is commented because there's a separate ticket to cover this work: OKTA-XXXXXX".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AnirudhMani-okta AnirudhMani-okta changed the title feat: add test selector for checkbox Add test selector for checkbox Jul 26, 2024
@AnirudhMani-okta AnirudhMani-okta changed the title Add test selector for checkbox Add Test Selector for Checkbox Jul 26, 2024
@AnirudhMani-okta AnirudhMani-okta force-pushed the am-OKTA-754187-add-checkbox-testselector branch from 0a5db23 to c15c07a Compare August 14, 2024 22:39
@AnirudhMani-okta AnirudhMani-okta force-pushed the am-OKTA-754187-add-checkbox-testselector branch 2 times, most recently from 58ff20c to d0faf57 Compare August 23, 2024 18:53
@AnirudhMani-okta AnirudhMani-okta force-pushed the am-OKTA-754187-add-checkbox-testselector branch from d0faf57 to e00b863 Compare August 26, 2024 21:59
@KevinGhadyani-Okta
Copy link
Contributor

The updated Test Selectors code is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants