kolibri icon indicating copy to clipboard operation
kolibri copied to clipboard

Wrap `KRadioButton` groups in `KRadioButtonGroup`

Open MisRob opened this issue 1 year ago • 15 comments

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

Summary

https://github.com/learningequality/kolibri-design-system/pull/650 introduces a new Kolibri Design System component, KRadioButtonGroup that ensures expected keyboard navigation in Firefox https://github.com/learningequality/kolibri/issues/10491.

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

References

  • Two first places are already updated in https://github.com/learningequality/kolibri/pull/12325
  • This will close https://github.com/learningequality/kolibri/issues/10491

Guidance

  • Find all <KRadioButton>s and wrap them in <KRadioButtonGroup> as described in https://develop--kolibri-design-system.netlify.app/kradiobutton/.
  • If the current Kolibri Design System release doesn't yet have KRadioButtonGroup available, you can use devserver-with-kds command to run Kolibri with your local KDS

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 avatar Aug 26 '24 09:08 MisRob

Hi, can I get assigned to this issue?

elkielki avatar Aug 28 '24 06:08 elkielki

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 https://github.com/learningequality/kolibri/pull/12616 and then run yarn install. Let us know if there are any issues.

MisRob avatar Aug 28 '24 09:08 MisRob

Hi is the issue still open?

adityashibu avatar Sep 01 '24 17:09 adityashibu

@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.

elkielki avatar Sep 01 '24 21:09 elkielki

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

AllanOXDi avatar Sep 02 '24 08:09 AllanOXDi

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?

adityashibu avatar Sep 02 '24 17:09 adityashibu

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 avatar Sep 03 '24 06:09 MisRob

@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 avatar Sep 03 '24 06:09 MisRob

Note that https://github.com/learningequality/kolibri/pull/12325 was just merged so the two first places are addressed in the latest develop. cc @adityashibu

MisRob avatar Sep 04 '24 21:09 MisRob

Sure I'll have a look at it :)

adityashibu avatar Sep 05 '24 07:09 adityashibu

Hi @adityashibu, are you working on this or planning to? Just to know if we should keep the assignment or not.

MisRob avatar Oct 03 '24 12:10 MisRob

Hey @MisRob can you assign me this issue?

iamshobhraj avatar Oct 10 '24 08:10 iamshobhraj

Hi @iamshobhraj, yes, thank you

MisRob avatar Oct 10 '24 15:10 MisRob

Hello , is this issue still open....if yes can u assign me?

Maku38 avatar Oct 16 '24 16:10 Maku38

@Maku38 this issue is currently assigned.

You’re welcome to find a "help wanted" issue with no assignee

bjester avatar Oct 16 '24 18:10 bjester

/assign

harshmohite04 avatar Oct 21 '24 21:10 harshmohite04

Hey @iamshobhraj. Someone is already assigned to this issue. If you want to work on something else, check out these other issues labeled help wanted" issue with no assignee

AllanOXDi avatar Oct 21 '24 21:10 AllanOXDi

@MisRob , I opened a pr for this issue

iamshobhraj avatar Oct 26 '24 22:10 iamshobhraj

Hi @iamshobhraj,

Thank you for your contribution! Our team will review the PR and provide feedback soon.

AllanOXDi avatar Oct 28 '24 13:10 AllanOXDi

Is this issue still open?

yashsaraswat2004 avatar Nov 04 '24 08:11 yashsaraswat2004

Hi @yashsaraswat2004 .It's already assigned. If you want to work on something else, check out these other issues labeled help wanted" issue with no assignee

AllanOXDi avatar Nov 04 '24 12:11 AllanOXDi

@MisRob, this issue should have been resolved by PR #12751. Is there anything further that needs to be addressed?

iamshobhraj avatar Dec 05 '24 23:12 iamshobhraj

Hi @iamshobhraj, thanks for the notice! Yes this is now closed by https://github.com/learningequality/kolibri/pull/12751

MisRob avatar Dec 06 '24 04:12 MisRob