Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap KRadioButton groups in KRadioButtonGroup #12596

Open
2 tasks
MisRob opened this issue Aug 26, 2024 · 10 comments
Open
2 tasks

Wrap KRadioButton groups in KRadioButtonGroup #12596

MisRob opened this issue Aug 26, 2024 · 10 comments
Assignees
Labels
DEV: frontend good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome

Comments

@MisRob
Copy link
Member

MisRob commented Aug 26, 2024

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

Summary

learningequality/kolibri-design-system#650 introduces a new Kolibri Design System component, KRadioButtonGroup that ensures expected keyboard navigation in Firefox #10491.

Now we need to update all places in Kolibri to use it.

References

Guidance

Acceptance criteria

  • There is no KRadioButton left in Kolibri that's not wrapped in KRadioButtonGroup
  • There are no regressions in radio button functionality in any of the updated places
@MisRob MisRob added DEV: frontend help wanted Open source contributors welcome good first issue Self-contained, straightforward, low-complexity labels Aug 26, 2024
@elkielki
Copy link

Hi, can I get assigned to this issue?

@MisRob
Copy link
Member Author

MisRob commented Aug 28, 2024

Hi @elkielki, thank you - yes. Please follow the guidance closely and ask here if anything comes up.

Also note that the newest Kolibri Design System that has <KRadioButtonGroup> available is not yet merged to Kolibri's develop and we currently have some issue with yarn linking these two repositories. I'd recommend that in your working branch, you change the two package.json's in the very same way as here #12616 and then run yarn install. Let us know if there are any issues.

@adityashibu
Copy link

Hi is the issue still open?

@elkielki
Copy link

elkielki commented Sep 1, 2024

@MisRob Feel free to reassign this issue to @adityashibu. I followed the 'Getting Started' documentation, but I'm stuck on the command nodeenv -p --node=18.19.0. For some reason, it's not installing npm.

@AllanOXDi AllanOXDi assigned adityashibu and unassigned elkielki Sep 2, 2024
@AllanOXDi
Copy link
Member

AllanOXDi commented Sep 2, 2024

Hi @elkielki I have unsigned you and assigned @adityashibu . Is there any error you are getting from the terminal that we can look at?

@adityashibu
Copy link

Hi @AllanOXDi, Just to make it clear, I need to wrap KRadioButton in KRadioButtonGroup right? So something along the lines of:

<KRadioButton
            v-for="language in languageCol"
            :key="language.id"
            ref="languageItem"
            v-model="selectedLanguage"
            :buttonValue="language.id"
            :label="language.lang_name"
            :title="language.english_name"
            class="language-name"
 />

would become:

<KRadioButtonGroup>
    <KRadioButton
                v-for="language in languageCol"
                :key="language.id"
                ref="languageItem"
                v-model="selectedLanguage"
                :buttonValue="language.id"
                :label="language.lang_name"
                :title="language.english_name"
                class="language-name"
     />
<KRadioButtonGroup />

Right?

@MisRob
Copy link
Member Author

MisRob commented Sep 3, 2024

Hi @adityashibu, yes. I'd ask you to study the guidance and all the links in the issue description where you will find plenty of examples of simple use-cases as well as more complex ones, see what's already done, documentation for components you will work with, and understand how to test that it works as expected in Firefox.

As you navigate Kolibri and preview the places you're refactoring, would you always please make a note about how you navigated as a user to a given place, and include this in your pull request description? It will help our QA team to locate where to test. Doesn't need to be elaborate, just few steps to get to each of those places. Thanks a lot.

@MisRob
Copy link
Member Author

MisRob commented Sep 3, 2024

@elkielki as @AllanOXDi mentioned, we're glad to help you debug your development server setup if you can give us some logs. Seems you're experiencing troubles with a node version managemen - many people have good experience with Volta.

@MisRob
Copy link
Member Author

MisRob commented Sep 4, 2024

Note that #12325 was just merged so the two first places are addressed in the latest develop. cc @adityashibu

@adityashibu
Copy link

Sure I'll have a look at it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DEV: frontend good first issue Self-contained, straightforward, low-complexity help wanted Open source contributors welcome
Projects
None yet
Development

No branches or pull requests

5 participants
@MisRob @elkielki @AllanOXDi @adityashibu and others