Skip to content

Conversation

@ivteplo
Copy link
Owner

@ivteplo ivteplo commented Jan 13, 2023

Related issue: #1

Things to do:

  • Create a BottomSheet class
  • Force the developer to specify an identifier for the sheet
  • Set up the aria-controls property for a button that closes a sheet
  • Set up the aria-controls property for a button that opens the sheet (just to make the example code better)
  • Find a way to limit access of screen reader and keyboard to the contents behind the sheet
  • Find a better way to generate the DOM
  • Rename CSS classes
  • Find a way to scope styles (so that they won't break user's layout)
  • Add more options to customize the sheet
  • Maybe create a custom tag as another way to use the component (?)
  • Implement custom events (hide/show)
  • Add a method to unmount the component and remove its event listeners from the window object
  • Add the title area in the top bar of the sheet - This can be a future enhancement
  • Write library documentation

@ivteplo ivteplo added the enhancement New feature or request label Jan 13, 2023
@ivteplo ivteplo self-assigned this Jan 13, 2023
@n-ce
Copy link

n-ce commented Apr 8, 2023

Hello, @ivteplo , I think you should drop the legendary ultimate DOM selector as it is a costly operation compared to just getElementById. It allows just as much readability but also the highest performance which is essential for a smooth ux.

@ivteplo ivteplo marked this pull request as draft April 8, 2023 19:45
@ivteplo
Copy link
Owner Author

ivteplo commented Sep 21, 2023

Maybe this will be useful to someone who is also interested in how to 'trap focus' inside of a modal:
https://css-tricks.com/a-css-approach-to-trap-focus-inside-of-an-element/

Now API is more universal. The sheet body won't be automatically replaced with the component code.
Instead, the library user should do it personally.

`aria-controls` was set to `""` in HTML.

The test has been updated according to the API changes.
Additionally, `<template>` is now used to store sheet body
(so that it won't be shown to the user in case the page will be loading for a long time)
@ivteplo
Copy link
Owner Author

ivteplo commented Dec 13, 2023

As far as I can see, focus trapping is quite a problematic feature. Probably, it would be better to let the user implement it instead (e.g. using a special library).

Why I consider this feature problematic to implement here:

  • Implementing focus trapping here myself would be somewhat excessive. Additionally, I would have to do a lot of testing.
  • Adding a direct dependency to a focus-trapping library might be a problem for people using the same library of a different version, since both versions would get included in the bundle and some incompatibility bugs might arise (especially when stacking multiple sheets or dialogs).
  • Adding a peer dependency to a focus-trapping library would force people follow my choice or, again, compatibility issues might arise.

I might create a tutorial on how to trap the focus inside the sheet with a separately-installed focus-trapping library (or libraries).

@ivteplo ivteplo marked this pull request as ready for review March 17, 2024 15:45
@ivteplo ivteplo merged commit 63138b6 into main Mar 17, 2024
@ivteplo ivteplo deleted the library branch March 17, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants