Make `KRadioButton` to show a warning for developers when it's not nested inside `KRadioButtonGroup`

MisRob commented 2 weeks ago

🌱 Are you new to the codebase? Welcome! Please see the contributing guidelines.

Summary

In https://github.com/learningequality/kolibri-design-system/pull/650, we introduced a new KRadioButtonGroup that ensures KRadioButton groups are accessible in all supported browsers. For this to properly work, all KRadioButtons need to be nested inside KRadioButtonGroup like

<KRadioButtonGroup>
  <KRadioButton>
  <KRadioButton>
</KRadioButtonGroup>

but it could also be more deeply nested structures such as

<KRadioButtonGroup>
  <KGridItem>
    <div>
      <KRadioButton>
      <KRadioButton>
    </div>
  </KGridItem>
</KRadioButtonGroup>

The goal of this issue is to add logic to KRadioButton that shows a warning for developers when it's not nested inside KRadioButtonGroup.

The Value Add

This will notify developers about the proper usage and ensures expected a11y behavior for users.

Possible Tradeoffs

A risk of possible performance issues since it adds logic to every KRadioButton that needs to traverse all its parents (see "Guidance" on how to possibly take care of this)

Guidance

You can use the playground page for development and testing
KRadioButton documentation page with more examples of usage
As demonstrated in the examples above, note that we can't know how deeply KRadioButton is nested in KRadioButtonGroup so we will need to look at its chain of parents until KRadioButtonGroup is found.
- However, as this will be run from every KRadioButton, we need to be careful to not use techniques of accessing the DOM that may cause performance issues.
- Recursion over parents in this.$parent and checking each $parent's $options._componentTag until we find 'KRadioButtonGroup' component tag or until there is no more parents, could be the way
Lastly, there's no need to run the check in production so let's wrap it in if (process.env.NODE_ENV !== 'production') {. @rtibbles says "this will also tree shake the code out in webpack builds." ;)

Acceptance criteria

[ ] No regressions in KRadioButtons
[ ] Optimized for performance
[ ] Executes in the development mode only
[ ] Works for one level of nesting as well as multiple levels and has unit tests for these two scenarios

Comments

This update would ideally be released after https://github.com/learningequality/kolibri/issues/12596 so that we're not overwhelmed by warnings in Kolibri ;)

lokesh-sagi125 commented 2 weeks ago

hey @MisRob can i work on this issue?

MisRob commented 1 week ago

Hi @lokesh-sagi125, you're welcome to take this on. Thanks for your continued contributions. Please follow guidance closely and you're welcome to ask questions here if there's anything that's not clear.

lokesh-sagi125 commented 1 week ago

hey @MisRob you want the warning to be displayed in the console right?

MisRob commented 1 week ago

Hi @lokesh-sagi125, yes

iamshobhraj commented 1 week ago

@MisRob can i work on this?

AlexVelezLl commented 1 week ago

Hey @iamshobhraj! Thanks for your interest in contributing to this issue. Unfortunately, this is already assigned. But you’re welcome to find a "help wanted" issue with no assignee :hugs:.

lokesh-sagi125 commented 1 day ago

hey @MisRob sorry for the inactivity i was busy with a hackathon ; i just opened a PR for this issue could you please review the changes :).

learningequality / kolibri-design-system