Skip to content

docs(Drawer): add API decision #2009

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

Merged
merged 12 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
147 changes: 147 additions & 0 deletions packages/blade/src/components/Drawer/_decisions/decisions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
# Drawer

A drawer is a panel that slides in mostly from right side of the screen over the existing content in the viewport. It helps in providing additional details or context and can also be used to promote product features or new products.

<img width="500px" src="./2024-02-07-20-04-07.png" alt="Drawer Figma Skeleton">

## Design
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will Drawer be absolute positioned always? What if it needs to be used as a left/right navigation drawer?

Copy link
Member Author

Choose a reason for hiding this comment

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

It will be fixed positioned on top right. It is always positioned like navigation drawer

Copy link
Contributor

Choose a reason for hiding this comment

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

So we are setting a default position for Drawers on the top-right across all products? Like we set bottom-left for toasts?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup

Copy link
Collaborator

Choose a reason for hiding this comment

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

There are use-cases where drawer is used for navigation on left side in a lot of apps. Won't that ever be a use-case for us?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Example:
image

Copy link
Member Author

Choose a reason for hiding this comment

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

There are use-cases where drawer is used for navigation on left side in a lot of apps. Won't that ever be a use-case for us?

Nope. So for desktop, our navigation bar itself is on left so there won't be anything opening from that side.

On mobile too we have drawers opening from right side only. Plus navigation component is something that we might tackle separately. This navigation component will use our Drawer internally on mobile. So at that time we can relook at this.

Anyway even if usecase comes up, its just addition of new prop so not much effort and no breaking change


- [Drawer - Figma Design](https://www.figma.com/file/jubmQL9Z8V7881ayUD95ps/Blade-DSL?node-id=78667%3A66663&mode=dev)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the drawer have a fixed width? Will the width change based on the content? If it is fixed width, why do we not have size options?

Copy link
Member Author

@saurabhdaware saurabhdaware Feb 8, 2024

Choose a reason for hiding this comment

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

It currently has a fixed max-width of 420px and inside that it takes width based on the content.

Let me check with design once if we want to go with max-width or defined size because in Modal I see its defined size

Copy link
Member Author

Choose a reason for hiding this comment

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

Just checked with RK. It is fixed width of 420px on desktop always. We will only support one size right now

Copy link
Collaborator

Choose a reason for hiding this comment

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

fixed width of 420px
Max width would be 420px right?

Copy link
Member Author

Choose a reason for hiding this comment

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

nope. Drawer is always 420px irrespective of the content.


## Proposed API

```jsx
<Drawer
isOpen={}
onDismiss={() => {}}
showOverlay
>
<DrawerHeader
title=""
subtitle=""
leading={<DrawerHeaderIcon />}
titleSuffix={<DrawerHeaderBadge />}
trailing={
<>
<DrawerHeaderLink />
<DrawerHeaderButton />
</>
}
/>
<DrawerBody>
<Slot />
</DrawerBody>
Copy link
Contributor

Choose a reason for hiding this comment

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

No Footer?

Copy link
Member Author

Choose a reason for hiding this comment

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

nope

<Drawer>
```

<details>
<summary><b>Full Usage Example</b></summary>

```jsx
const MyCuteDrawer = () => {
const [showDrawer, setShowDrawer] = React.useState(false);
return (
<Box>
<Button onClick={() => setShowDrawer(true)}>Open Drawer</Button>
<Drawer
isOpen={showDrawer}
onDismiss={() => {
setShowDrawer(false);
}}
>
<DrawerHeader
title="Announcements"
/>
<DrawerBody>
<FTXAnnouncement />
<RazorpayOnePromotions />
<CatPictures />
</DrawerBody>
<Drawer>
</Box>
)
}

```

</details>

### Props

```ts
type DrawerProps = {
/**
* Controlled state of drawer open or not
*/
isOpen: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if multiple drawers are opened together? I'm guessing same behaviour as Modal?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes similar but only 2 drawers can be kept open at once from design-side. And the 2nd drawer has a back button instead of close button.

Let me add the stacking behaviour to this API doc I didn't mention it in this

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would we need a separate callback for the back button?

Copy link
Member Author

Choose a reason for hiding this comment

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

back button is just dismiss of 2nd Drawer. Consumer knows which is their second drawer so they can use that onDismiss itself.


/**
* Dismiss handler
*/
onDismiss: () => void;

/**
* Show or hide overlay.
*
* Also decides if clicking outside on overlay closes the drawer or not
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wont there ever be a possibility where overlay isn't needed but click outside still needs to close the drawer?

Copy link
Member Author

Choose a reason for hiding this comment

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

no I did ask this to designers. If someone wants clicking outside to close drawer, they should always have overlay

Copy link
Contributor

Choose a reason for hiding this comment

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

Also decides if clicking outside on overlay closes the drawer or not

I feel we should have a separate prop for deciding this, it shouldn't be dependant on the presence of overlay.

Copy link
Member Author

Choose a reason for hiding this comment

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

you mean for a usecase where someone doesn't want to show overlay but still want to close Drawer on clicking outside? I can confirm this once with design if this should be possible. Ideally overlay should be present in most cases except few exceptional cases where outer items have to be interactive

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also vote for the separation of the logic to dismiss the overlay items on click outside on a different prop. If Modal and BottomSheet have a prop tomorrow to disable dismiss on click outside on the overlay the same prop should be extended over here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. Better to have a separate prop since we might be adding the same to Modal & BottomSheet too

Copy link
Member Author

Choose a reason for hiding this comment

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

Modal and BottomSheet would anyway have different prop than Drawer in this particular case no?

E.g. in Modal and BottomSheet, we always want to show overlay so there will never be a showOverlay prop. Whereas in Drawer, we definitely need to have showOverlay prop.

If we're having that prop and we know interactions outside have to be consistent with display of overlay, wouldn't adding new prop be extra overhead for consumers here?

Because if we add 2 props, it creates impossible states of

<>
  <Drawer
    showOverlay={true}
    shouldDismissOnOutsideClick={false}
  />
  <Drawer
    showOverlay={false}
    shouldDismissOnOutsideClick={true}
  />
</>

These scenarios should throw error in 2 props case because its not correct usage from UX perspective

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh anyway just discussed this with design - https://razorpay.slack.com/archives/G01B3LQ9H0W/p1707808893726369

Usecase that we have on Modal is something that we would have it here as well. I'll make changes to add 2 props.

*/
showOverlay?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

So overlay is optional? Won't that look weird? What is the use case?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have usecases where Drawer is used for Announcements. In those cases, users should still be able to interact with dashboard items even with Drawer open so we cant show overlay or hide drawer on clicking outside.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

when overlay is disabled, does the area outside the drawer remain interactive?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Copy link
Collaborator

Choose a reason for hiding this comment

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

does the area outside the drawer remain interactive

We should ideally control this with a new prop like shouldDismissOnClickOutside

};

type DrawerHeaderProps = {
/**
* Title of the Drawer
*/
title: string;

/**
* Subtitle of the Drawer
*/
subtitle?: string;

/**
* Leading element
*
* DrawerHeaderIcon or DrawerHeaderAsset
*/
leading?: ReactNode;

/**
* Title suffix element
*
* DrawerHeaderBadge
*/
titleSuffix?: ReactNode;

/**
* Title trailing element
*
* DrawerHeaderLink, DrawerHeaderButton[]
*/
trailing?: ReactNode;
};
```

Other supporting wrapper components for trailing and leading space-

- DrawerHeaderBadge
- DrawerHeaderIcon
- DrawerHeaderAsset
- DrawerHeaderLink
- DrawerHeaderButton
Copy link
Collaborator

Choose a reason for hiding this comment

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

We keep creating these components for multiple components... Modal, Bottomsheet, Drawer. We might need to look into changing these into just HeaderBadge, HeaderIcon, etc. This would keep adding unnecessary repetitive 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.

true. HeaderBadge is a good idea. Lets discuss this with team once in next standup?

Copy link
Contributor

Choose a reason for hiding this comment

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

How are these components different from normal Badge/Icon? For example, I see _CardHeaderBadge is same as Badge. Why can't we use theBadge component directly?

const _CardHeaderBadge = (props: BadgeProps): React.ReactElement => {

Copy link
Member Author

Choose a reason for hiding this comment

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

These subcomponents usually have defined defaults that are different from defaults of actual component. E.g.

return <Badge size="medium" marginLeft="spacing.3" {...props} />;

However in some examples like CardHeaderBadge, the defaults of wrapper are same as defaults of actual component. There it is just for consistency in API otherwise some components will be like Badge while some be like CardHeaderText etc


_No alternate APIs were considered because Drawer is closer to Modal on overall meaning and API perspective so made sense to go with API that is closer to Modal, also all DS Drawer components I referenced have similar API_

## Accessibility

- **Aria Attributes:** Drawer will have `aria-modal="true"` and `role="dialog"` and will be treated as modal for voiceover users.
- **Focus Handling:** Ensure Close Icon is focussable
Copy link
Collaborator

Choose a reason for hiding this comment

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

Like Modal & Bottomsheet, should we also expose initialFocusRef?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I think we should. Drawer will also have similar usecases like Modal and BottomSheet so requirement will anyway come

Copy link
Contributor

Choose a reason for hiding this comment

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

Add keyboard handling details, e.g, Esc should close the drawer.

Copy link
Member Author

Choose a reason for hiding this comment

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

done


Will work in a similar manner as [Ant Design - Drawer](https://ant.design/components/drawer)

## References

- https://atlassian.design/components/drawer/examples
- https://ant.design/components/drawer

## Open Questions