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

feat(Drawer): Documentation & Tests #2015

Merged
merged 63 commits into from
Feb 26, 2024
Merged

Conversation

saurabhdaware
Copy link
Member

@saurabhdaware saurabhdaware commented Feb 14, 2024

Documentation Link

https://drawer-docs-tests--61c19ee8d3d282003ac1d81c.chromatic.com/?path=/story/components-drawer

Description

Changes

  • Documentation
  • E2E
  • Unit Test
  • Snapshot Updates

Additional Information

No

Component Checklist

  • Update Component Status Page
  • Perform Manual Testing in Other Browsers
  • Add KitchenSink Story
  • Add Interaction Tests (if applicable)
  • Add changeset

saurabhdaware and others added 30 commits February 7, 2024 21:15
DrawerOpen.play = async () => {
const { getByRole, queryByRole, getByTestId, queryByTestId } = within(document.body);

await expect(queryByRole('heading', { name: 'Drawer Heading' })).not.toBeInTheDocument();
Copy link
Member

Choose a reason for hiding this comment

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

put the texts in variables and reuse them

await waitFor(() => expect(getByRole('dialog')).toBeVisible());
await user.click(getByRole('button', { name: 'Close' }));
await waitFor(() => expect(queryByRole('dialog')).not.toBeInTheDocument());
expect(onDismiss).toHaveBeenCalledTimes(1);
Copy link
Member

Choose a reason for hiding this comment

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

We can also test this in interaction tests, see carousel interaction tests

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why but its not working as expected in interaction test. Like the onDismissMock is getting called but something going wrong with referrencing variable from global scope I guess. The onDismissMock reference getting called might be different from onDiss

image

Anyways I think this should be tested on unit test since on a unit you're checking if your prop and handlers work or not.

On interaction I have added state handling that eventually ensures drawer is getting closed or not when the close state is handled

expect(onDismiss).toHaveBeenCalledTimes(1);
});

it('should dismiss on overlay click', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this, if this case is already covered via interaction tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to keep some basic functionality tests on unit test as well since interaction tests we can miss to run. So unit test only covers open close handlers and critical dismiss behaviour

export default {
title: 'Components/Drawer',
component: Drawer,
subcomponents: { Drawer, DrawerHeader, DrawerBody },
Copy link
Member

Choose a reason for hiding this comment

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

wow TIL

Copy link
Member

Choose a reason for hiding this comment

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

We need to do this for all overlay components.

Copy link
Member Author

Choose a reason for hiding this comment

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

The con of this is that the storybook controls don't work with such components. With Drawer it seems fine since it doesn't have that many props and I have added interactive examples but don't think so we can do it on other components

@@ -12,7 +12,7 @@ const getBladeVersion = (): string => {
return `https://pkg.csb.dev/razorpay/blade/commit/${shortSha}/@razorpay/blade`;
}

return '*';
return 'https://pkg.csb.dev/razorpay/blade/commit/a168a460/@razorpay/blade';
Copy link
Member

Choose a reason for hiding this comment

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

TODO: revert

Copy link
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM

@saurabhdaware saurabhdaware added Review - L3 Third level of review and removed Review - L1 First level of review labels Feb 26, 2024
Base automatically changed from drawer/implementation to drawer/integration February 26, 2024 10:48
@saurabhdaware saurabhdaware merged commit 1cf00bc into drawer/integration Feb 26, 2024
12 of 14 checks passed
@saurabhdaware saurabhdaware deleted the drawer/docs-tests branch February 26, 2024 11:10
saurabhdaware added a commit that referenced this pull request Feb 27, 2024
* feat(Drawer): add Drawer component (#2011)

* feat(Drawer): add API decisions

* feat: add full skeleton

* fix: image width

* fix: typo

* feat: add base implementation, move zIndices to utils

* fix: animation speed

* refactor: move to different files

* feat: make animations smooth

* feat: add fancy header in example

* fix: link and button instances

* feat: change animation speed from xgentle to gently

* feat: add stacking section to api decision

* Update packages/blade/src/components/Drawer/_decisions/decisions.md

Co-authored-by: Nitin Kumar <[email protected]>

* fix: focus management on back button

* feat: add native warning

* fix: focus of back and close button

* refactor: move stack provider to drawer directory

* fix: remove DrawerHeaderLink and DrawerHeaderButton

* fix: linting issue

* fix: header alignments and button size

* feat: add open questions

* feat: add discussion slack thread

* feat: update API decision with new API

* feat: build for new API

* fix: z-index and stacking animations

* fix: spacing between header icon and title

* fix: rn error

* fix: ts errors

* feat: export Drawer

* fix: allow scrollable content

* Update packages/blade/src/components/Drawer/DrawerSubcomponents.native.tsx

Co-authored-by: Nitin Kumar <[email protected]>

* Update packages/blade/src/components/Drawer/DrawerSubcomponents.native.tsx

Co-authored-by: Nitin Kumar <[email protected]>

---------

Co-authored-by: Nitin Kumar <[email protected]>

* feat(Drawer): Documentation & Tests (#2015)

* feat(Drawer): add API decisions

* feat: add full skeleton

* fix: image width

* fix: typo

* feat: add base implementation, move zIndices to utils

* fix: animation speed

* refactor: move to different files

* feat: make animations smooth

* feat: add fancy header in example

* fix: link and button instances

* feat: change animation speed from xgentle to gently

* feat: add stacking section to api decision

* Update packages/blade/src/components/Drawer/_decisions/decisions.md

Co-authored-by: Nitin Kumar <[email protected]>

* fix: focus management on back button

* docs: add basic page

* feat: add native warning

* fix: focus of back and close button

* refactor: move stack provider to drawer directory

* fix: remove DrawerHeaderLink and DrawerHeaderButton

* fix: linting issue

* fix: args

* fix: header alignments and button size

* feat: add Drawer examples

* feat: add open questions

* feat: add discussion slack thread

* feat: add subcomponent prop

* feat: add no overlay story

* feat: update API decision with new API

* feat: build for new API

* fix: drawer api

* fix: z-index and stacking animations

* fix: spacing between header icon and title

* fix: rn error

* fix: ts errors

* feat: export Drawer

* feat: add scrollable content example

* fix: allow scrollable content

* docs: add complex examples

* feat: add basic e2e

* feat: add e2e

* tests: add unit tests

* tests: update snapshots

* feat: add kitchen sink

* feat: add story with kitchen sink

* fix: flaky test

* feat: resolve jsdoc grammer

* Update packages/blade/src/components/Drawer/DrawerSubcomponents.native.tsx

Co-authored-by: Nitin Kumar <[email protected]>

* Update packages/blade/src/components/Drawer/DrawerSubcomponents.native.tsx

Co-authored-by: Nitin Kumar <[email protected]>

* feat: add jsdoc for testid

* feat: revert blade version

* empty

---------

Co-authored-by: Nitin Kumar <[email protected]>

* Create mean-comics-flash.md

---------

Co-authored-by: Nitin Kumar <[email protected]>
anuraghazra pushed a commit that referenced this pull request Apr 9, 2024
* feat(Drawer): add Drawer component (#2011)

* feat(Drawer): add API decisions

* feat: add full skeleton

* fix: image width

* fix: typo

* feat: add base implementation, move zIndices to utils

* fix: animation speed

* refactor: move to different files

* feat: make animations smooth

* feat: add fancy header in example

* fix: link and button instances

* feat: change animation speed from xgentle to gently

* feat: add stacking section to api decision

* Update packages/blade/src/components/Drawer/_decisions/decisions.md

Co-authored-by: Nitin Kumar <[email protected]>

* fix: focus management on back button

* feat: add native warning

* fix: focus of back and close button

* refactor: move stack provider to drawer directory

* fix: remove DrawerHeaderLink and DrawerHeaderButton

* fix: linting issue

* fix: header alignments and button size

* feat: add open questions

* feat: add discussion slack thread

* feat: update API decision with new API

* feat: build for new API

* fix: z-index and stacking animations

* fix: spacing between header icon and title

* fix: rn error

* fix: ts errors

* feat: export Drawer

* fix: allow scrollable content

* Update packages/blade/src/components/Drawer/DrawerSubcomponents.native.tsx

Co-authored-by: Nitin Kumar <[email protected]>

* Update packages/blade/src/components/Drawer/DrawerSubcomponents.native.tsx

Co-authored-by: Nitin Kumar <[email protected]>

---------

Co-authored-by: Nitin Kumar <[email protected]>

* feat(Drawer): Documentation & Tests (#2015)

* feat(Drawer): add API decisions

* feat: add full skeleton

* fix: image width

* fix: typo

* feat: add base implementation, move zIndices to utils

* fix: animation speed

* refactor: move to different files

* feat: make animations smooth

* feat: add fancy header in example

* fix: link and button instances

* feat: change animation speed from xgentle to gently

* feat: add stacking section to api decision

* Update packages/blade/src/components/Drawer/_decisions/decisions.md

Co-authored-by: Nitin Kumar <[email protected]>

* fix: focus management on back button

* docs: add basic page

* feat: add native warning

* fix: focus of back and close button

* refactor: move stack provider to drawer directory

* fix: remove DrawerHeaderLink and DrawerHeaderButton

* fix: linting issue

* fix: args

* fix: header alignments and button size

* feat: add Drawer examples

* feat: add open questions

* feat: add discussion slack thread

* feat: add subcomponent prop

* feat: add no overlay story

* feat: update API decision with new API

* feat: build for new API

* fix: drawer api

* fix: z-index and stacking animations

* fix: spacing between header icon and title

* fix: rn error

* fix: ts errors

* feat: export Drawer

* feat: add scrollable content example

* fix: allow scrollable content

* docs: add complex examples

* feat: add basic e2e

* feat: add e2e

* tests: add unit tests

* tests: update snapshots

* feat: add kitchen sink

* feat: add story with kitchen sink

* fix: flaky test

* feat: resolve jsdoc grammer

* Update packages/blade/src/components/Drawer/DrawerSubcomponents.native.tsx

Co-authored-by: Nitin Kumar <[email protected]>

* Update packages/blade/src/components/Drawer/DrawerSubcomponents.native.tsx

Co-authored-by: Nitin Kumar <[email protected]>

* feat: add jsdoc for testid

* feat: revert blade version

* empty

---------

Co-authored-by: Nitin Kumar <[email protected]>

* Create mean-comics-flash.md

---------

Co-authored-by: Nitin Kumar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants