✨ Implement <RadioGroup /> component
#143
Conversation
mofiebiger
changed the title
<RadioGroup /> component
mofiebiger
force-pushed
the
feature/radio-group
branch
2 times, most recently
from
8133369 to
7f1f661
Compare
mofiebiger
force-pushed
the
feature/radio-group
branch
from
70b269c to
cfaf9a7
Compare
mofiebiger
force-pushed
the
feature/radio-group
branch
from
cfaf9a7 to
cee3ed9
Compare
dmcnamara-eng
left a comment
dmcnamara-eng
left a comment
There was a problem hiding this comment.
Hey @mofiebiger I've made one request for changes, but this looks almost ready to merge 🙌
addon/components/radio-group.hbs
Outdated
| role='radiogroup' | ||
| aria-labelledby={{this.radioGroupLabel}} | ||
| aria-disabled={{@disabled}} | ||
| {{did-insert this.setTabIndex}} |
There was a problem hiding this comment.
Hey @mofiebiger did-insert can be considered harmful. Could you convert this to a modifier?
See:
https://nullvoxpopuli.com/avoiding-lifecycle/
https://github.com/GavinJoyce/ember-headlessui/pull/116/files
There was a problem hiding this comment.
Also, if someone would be willing to totally remove @ember/render-modifiers, that would be a huge help <3
There was a problem hiding this comment.
@NullVoxPopuli is that best done once you've sorted the monorepo? If not, on which branch? Happy to take that job on either way.
There was a problem hiding this comment.
I think git is smart enough to figure it out, actually. shouldn't be too bad. could happen on any branch. no need to wait on me <3
There was a problem hiding this comment.
@dmcnamara-eng, the did-insert and will-destroy lifecycle hooks are now converted to modifiers. Please see changes here: 291c4da
mofiebiger
force-pushed
the
feature/radio-group
branch
from
cee3ed9 to
291c4da
Compare
5 participants
✨ What Changed and Why
This PR is a functionally complete implementation of a
<RadioGroup />component based on headlessui.dev/react/radio-groupThings that this PR implements:
😈 Before
NA
😇 After
headlessui-radio-group.mp4
🌈 Testing
phorest:feature/radio-groupCloses: #103