staccx / bento

https://bento.stac.cx
5 stars 1 forks source link

Replace expand #44

Open XSlemX opened 3 years ago

XSlemX commented 3 years ago

The problem

is pretty weird. ## Environment * Bento version (or git revision) that exhibits the issue: latest * Last Bento version that did not exhibit the issue (if applicable): n/a * Node.js version: any * Npm or Yarn package manager: any * Mobile platform/version under test: ## Details Firstly it uses the 1st child as its button. Secondly it conditionally renders children based on the expanded flag. This causes any local state to be lost (forms, useState etc). This should be rewritten or replaced. render should not be conditional and should just dump children. When !expanded it should `hideVisually` content wrapper: ```jsx
expandedSet(!expanded)} title={title}> {titleComponent} {children}
``` The title component should be a prop and not first child. `React.Children.map` is so 2018. ## Console and log errors If applicable, please include any console or log errors. ## Code To Reproduce Issue [ Good To Have ] Please share relevant sample code. Or better yet, provide a link to a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example).
robinsandborg commented 3 years ago

Not sure if hideVisually is the best solution in terms of accessibility, souls be investigated. Is Aria labels + display: none better?

Thymas1 commented 3 years ago

solution: https://github.com/staccx/bento/pull/45