Skip to content

feat: Add reusable BottomSheet component#1653

Open
KhushamBansal wants to merge 3 commits into
layer5io:masterfrom
KhushamBansal:feat/bottomsheet
Open

feat: Add reusable BottomSheet component#1653
KhushamBansal wants to merge 3 commits into
layer5io:masterfrom
KhushamBansal:feat/bottomsheet

Conversation

@KhushamBansal

Copy link
Copy Markdown
Member

Notes for Reviewers

This PR fixes #

Screen.Recording.2026-06-23.at.1.38.19.PM.mov

Signed commits

  • Yes, I signed my commits.

Signed-off-by: KhushamBansal <kbkhushambansal@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new mobile-friendly BottomSheet component that slides up from the bottom of the screen. The feedback recommends exposing the close button's aria-label as a configurable prop (closeButtonAriaLabel) instead of hardcoding the string 'Close' to support internationalization (i18n) and localization.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/custom/BottomSheet/BottomSheet.tsx Outdated
Comment thread src/custom/BottomSheet/BottomSheet.tsx Outdated
Signed-off-by: KhushamBansal <kbkhushambansal@gmail.com>
Signed-off-by: KhushamBansal <kbkhushambansal@gmail.com>

@leecalcote leecalcote left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Goooooood work here. 🥇 Let's reinforce this work and make it super solid, defined once, reused everywhere by ensuring:

  1. no deviances in its use should be necessary; no maintenance need in BottomSheet or in the components that use it. This should be a one-time effort. For example, given its use in the display of mui-datatable column checkboxes or mui-datatable filter choices, the instances of those mui-datatables should not need to wire up to the BottomSheet - this shall be dynamic; neither component should require no consideration on an ongoing basis in either BottomSheet or in mui-datatables.
  2. How does this compare to what we have in meshery-cloud on the /catalog page?
  3. Is there an open issue to migrate that implementation to a centralized component?
  4. What learnings are being taken from the component in meshery-cloud?
  5. In what ways is this the best possible version of a "BottomSheet" component? Were are our examples of mui-datatables using this component to display filters?
  6. Is there an open issue to centrally and pervasively incorporate this BottomSheet component into use of each and every mui-dtatatable and to have code written once-only that defines when to activate and deactivate the BottomSheet (so the mobile experience is simply implicit)?

Answer here and/or link to each of the GH issues that you open in response to these items. Don't merge this unless my requirements above are being met.

@KhushamBansal

KhushamBansal commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

@leecalcote @aabidsofi19
Some points I want to highlight. Please correct me if I am wrong.

  • (answer 1) I hadn't considered mui-datatables/meshery-cloud earlier, as I considered the fix only for Registry (both meshery-ui and Kanvas).
  • After going through the codebase now, I found that the mui-datatable filters work perfectly fine on mobile viewports (though the UI can be improved), as they use popover.js for their internal filter/column menus (screenshot attached).
Screenshot 2026-06-24 at 12 46 10 PM
  • (answer 2,4) I have only read access to meshery-cloud. Could you provide me with access to clone the repository? It will make it easier for me to explore the codebase as we can search only specific files using github search and not components/keywords.

Conclusion

  • (answer 1, 5,6) We need to add mobile viewport support in MeshModelComponent.tsx, as it uses MesheryTreeView. We also need a BottomSheet in MeshModelDetails for mobile view. So, mui-datatable is not used here.
  • (answer 3) No
  • mui-datatables already handles mobile viewports. So, no need to open new issue for this.
  • BottomSheet was designed to display component details in a popup instead of Panel-2, as layout was broken on mobile viewports.
Screenshot 2026-06-24 at 12 49 02 PM

@leecalcote

Copy link
Copy Markdown
Member

Consider using the same approach used for other components.

@KhushamBansal

Copy link
Copy Markdown
Member Author

Thanks @leecalcote for your suggestion. I found that we can also use the <Modal> component to display the registry component details, as it's already used in Meshery UI. I've attached a demo video.
Please let me know which approach you prefer: BottomSheet (demo in the PR description) or Modal (demo attached).

If you think we should go ahead with <Modal>, I'll close this PR.

Screen.Recording.2026-06-25.at.12.23.30.PM.mov

@Bhumikagarggg

Copy link
Copy Markdown
Contributor

@KhushamBansal Thank you for your contribution! Let's discuss this during the website call tomorrow at 5:30 PM IST | 7 AM CST Add it as an agenda item to the meeting minutes, if you would 🙂

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.

4 participants