refac: change `Checkbox` to be a wrapper around `KCheckbox` instead of `VCheckbox` #4472

EshaanAgg · 2024-03-08T19:59:06Z

Summary

Description of the change(s) you made

Changes the Checkbox component to make use of the KCheckbox internally and adds support for using v-model to the same
The v-model is designed to handle three kinds of values: arrays, number, and boolean

Manual verification steps performed

Ran studio after making all the changes and verified manually if the checkboxes were behaving as expected.

Does this introduce any tech-debt items?

No, but this PR must be manually tested before it can be merged. This is because all the occurrences of the Checkbox right now have props and values recognized by VCheckbox. These props need to be added to our new implementation of Checkbox as well or need to be handled in the components using them separately.

Reviewer guidance

How can a reviewer test these changes?

Running the application locally.

Are there any risky areas that deserve extra testing?

All checkbox-related menus and forms.

Contributor's Checklist

PR process:

If this is an important user-facing change, PR or related issue the CHANGELOG label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later time
If this includes an internal dependency change, a link to the diff is provided
The docs label has been added if this introduces a change that needs to be updated in the user docs?
If any Python requirements have changed, the updated requirements.txt files also included in this PR
Opportunities for using Google Analytics here are noted
Migrations are safe for a large db

Studio-specifc:

All user-facing strings are translated properly
The notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)
All UI components are LTR and RTL compliant
Views are organized into pages, components, and layouts directories as described in the docs
Users' storage used is recalculated properly on any changes to main tree files
If there new ways this uses user data that needs to be factored into our Privacy Policy, it has been noted.

Testing:

Code is clean and well-commented
Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Any new interactions have been added to the QA Sheet
Critical and brittle code paths are covered by unit tests

Reviewer's Checklist

This section is for reviewers to fill out.

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

MisRob · 2024-03-12T11:40:32Z

Thanks @EshaanAgg!

For reference for reviewers, this PR is not coming out of the blue :) @EshaanAgg has discussed high-level approach with @bjester and me (https://learningequality.slack.com/archives/C03S2EN192A/p1709751447068349)

bjester · 2024-03-12T18:46:18Z

@EshaanAgg Thanks for working on this! Seems there are some tests failing. They might need updated, but I think there's a possibility the Checkbox abstraction may need to handle objects too, i.e. setting a boolean for a key in an object.

Here's the summary of the failing tests (the link may not properly open and scroll to it, so I've copied the output below):
https://github.com/learningequality/studio/actions/runs/8208392765/job/22577441251?pr=4472#step:6:12869

You can run the tests locally with yarn run test-jest

FAIL contentcuration/contentcuration/frontend/channelEdit/components/AnswersEditor/AnswersEditor.spec.js
  ● AnswersEditor › for a multiple selection question › renders only correct answers as checked

    expect(received).toBe(expected) // Object.is equality

    Expected: true
    Received: false

      142 |       const inputs = wrapper.findAll('input');
      143 |
    > 144 |       expect(inputs.at(0).element.checked).toBe(true);
          |                                            ^
      145 |       expect(inputs.at(1).element.checked).toBe(false);
      146 |       expect(inputs.at(2).element.checked).toBe(true);
      147 |     });

      at Object.<anonymous> (contentcuration/contentcuration/frontend/channelEdit/components/AnswersEditor/AnswersEditor.spec.js:144:44)

FAIL contentcuration/contentcuration/frontend/channelEdit/components/AssessmentEditor/AssessmentEditor.spec.js
  ● AssessmentEditor › renders answers preview on show answers click

    expect(received).toBe(expected) // Object.is equality

    Expected: true
    Received: false

      224 |     const items = getItems(wrapper);
      225 |
    > 226 |     expect(isAnswersPreviewVisible(items.at(0))).toBe(true);
          |                                                  ^
      227 |     expect(isAnswersPreviewVisible(items.at(1))).toBe(true);
      228 |     expect(isAnswersPreviewVisible(items.at(2))).toBe(true);
      229 |     expect(isAnswersPreviewVisible(items.at(3))).toBe(true);

      at Object.<anonymous> (contentcuration/contentcuration/frontend/channelEdit/components/AssessmentEditor/AssessmentEditor.spec.js:226:50)
          at runMicrotasks (<anonymous>)

FAIL contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js
  ● contentDefaults › initial state › should fill fields with provided content defaults

    expect(received).toEqual(expected) // deep equality

    Expected: false
    Received: undefined

      16 |   keys.forEach(key => {
      17 |     const field = fields.filter(f => f.contains(`[data-name="${camelCase(key)}"]`)).at(0);
    > 18 |     expect(field.props(prop)).toEqual(contentDefaults[key]);
         |                               ^
      19 |   });
      20 | }
      21 |

      at forEach (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:18:31)
          at Array.forEach (<anonymous>)
      at assertFieldValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:16:8)
      at assertFormValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:59:3)
      at Object.<anonymous> (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:117:7)
          at runMicrotasks (<anonymous>)

  ● contentDefaults › initial state › should fill fields with default values when no content defaults provided

    expect(received).toEqual(expected) // deep equality

    Expected: true
    Received: undefined

      16 |   keys.forEach(key => {
      17 |     const field = fields.filter(f => f.contains(`[data-name="${camelCase(key)}"]`)).at(0);
    > 18 |     expect(field.props(prop)).toEqual(contentDefaults[key]);
         |                               ^
      19 |   });
      20 | }
      21 |

      at forEach (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:18:31)
          at Array.forEach (<anonymous>)
      at assertFieldValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:16:8)
      at assertFormValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:59:3)
      at Object.<anonymous> (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:122:7)
          at runMicrotasks (<anonymous>)

  ● contentDefaults › initial state › should pre-validate license value

    expect(received).toEqual(expected) // deep equality

    Expected: true
    Received: undefined

      16 |   keys.forEach(key => {
      17 |     const field = fields.filter(f => f.contains(`[data-name="${camelCase(key)}"]`)).at(0);
    > 18 |     expect(field.props(prop)).toEqual(contentDefaults[key]);
         |                               ^
      19 |   });
      20 | }
      21 |

      at forEach (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:18:31)
          at Array.forEach (<anonymous>)
      at assertFieldValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:16:8)
      at assertFormValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:59:3)
      at Object.<anonymous> (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:129:7)
          at runMicrotasks (<anonymous>)

  ● contentDefaults › updating state › should update fields with new content defaults received from a parent

    expect(received).toEqual(expected) // deep equality

    Expected: false
    Received: undefined

      16 |   keys.forEach(key => {
      17 |     const field = fields.filter(f => f.contains(`[data-name="${camelCase(key)}"]`)).at(0);
    > 18 |     expect(field.props(prop)).toEqual(contentDefaults[key]);
         |                               ^
      19 |   });
      20 | }
      21 |

      at forEach (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:18:31)
          at Array.forEach (<anonymous>)
      at assertFieldValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:16:8)
      at assertFormValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:59:3)
      at Object.<anonymous> (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:148:7)
          at runMicrotasks (<anonymous>)

  ● contentDefaults › updating state › should update a parent with new content defaults

    [vue-test-utils]: find did not return .v-input [data-name="autoDeriveAudioThumbnail"], cannot call is() on empty Wrapper

      26 |     const input = field.find(`.v-input ${selector}`);
      27 |
    > 28 |     if (input.is('[type="checkbox"]')) {
         |               ^
      29 |       input.setChecked(contentDefaults[key]);
      30 |     } else {
      31 |       input.setValue(contentDefaults[key]);

      at throwError (node_modules/@vue/test-utils/dist/vue-test-utils.js:1417:9)
      at ErrorWrapper.is (node_modules/@vue/test-utils/dist/vue-test-utils.js:2180:3)
      at forEach (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:28:15)
          at Array.forEach (<anonymous>)
      at updateFieldValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:23:8)
      at updateFormValues (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:89:3)
      at Object.<anonymous> (contentcuration/contentcuration/frontend/shared/views/form/__tests__/contentDefaults.spec.js:176:7)
          at runMicrotasks (<anonymous>)

FAIL contentcuration/contentcuration/frontend/channelEdit/views/trash/__tests__/trashModal.spec.js (5.003 s)
  ● trashModal › on topic tree selection › checking item in list should set selected

    expect(received).toEqual(expected) // deep equality

    - Expected  - 3
    + Received  + 1

    - Array [
    -   "selected",
    - ]
    + Array []

      93 |     it('checking item in list should set selected', () => {
      94 |       wrapper.find('[data-test="checkbox"]').vm.$emit('change', ['selected']);
    > 95 |       expect(wrapper.vm.selected).toEqual(['selected']);
         |                                   ^
      96 |     });
      97 |     it('checking select all checkbox should check all items', () => {
      98 |       wrapper.find('[data-test="selectall"]').vm.$emit('change', true);

      at Object.<anonymous> (contentcuration/contentcuration/frontend/channelEdit/views/trash/__tests__/trashModal.spec.js:95:35)
          at runMicrotasks (<anonymous>)

FAIL contentcuration/contentcuration/frontend/channelEdit/components/edit/__tests__/accessibilityOptions.spec.js
  ● AccessibilityOptions › should render appropriate tooltips along with the checkbox

    expect(received).toBe(expected) // Object.is equality

    Expected: true
    Received: false

      93 |
      94 |     expect(wrapper.find('[data-test="checkbox-altText"]').exists()).toBe(true);
    > 95 |     expect(wrapper.find('[data-test="tooltip-altText"]').exists()).toBe(true);
         |                                                                    ^
      96 |     expect(wrapper.find('[data-test="checkbox-highContrast"]').exists()).toBe(true);
      97 |     expect(wrapper.find('[data-test="tooltip-highContrast"]').exists()).toBe(true);
      98 |     expect(wrapper.find('[data-test="checkbox-taggedPdf"]').exists()).toBe(true);

      at Object.<anonymous> (contentcuration/contentcuration/frontend/channelEdit/components/edit/__tests__/accessibilityOptions.spec.js:95:68)
          at runMicrotasks (<anonymous>)

FAIL contentcuration/contentcuration/frontend/channelList/views/ChannelSet/__tests__/channelSelectionList.spec.js
  ● channelSelectionList › should select channels when the channel has been checked

    TypeError: Cannot read properties of undefined (reading '0')

      62 |     wrapper.setData({ loading: false });
      63 |     wrapper.find('[data-test="checkbox"]').vm.$emit('change', [editChannel.id]);
    > 64 |     expect(wrapper.emitted('input')[0][0]).toEqual([editChannel.id]);
         |            ^
      65 |   });
      66 |   it('should deselect channels when the channel has been unchecked', () => {
      67 |     wrapper.setData({ loading: false });

      at Object.<anonymous> (contentcuration/contentcuration/frontend/channelList/views/ChannelSet/__tests__/channelSelectionList.spec.js:64:12)
          at runMicrotasks (<anonymous>)

  ● channelSelectionList › should deselect channels when the channel has been unchecked

    TypeError: Cannot read properties of undefined (reading '0')

      67 |     wrapper.setData({ loading: false });
      68 |     wrapper.find('[data-test="checkbox"]').vm.$emit('change', []);
    > 69 |     expect(wrapper.emitted('input')[0][0]).toEqual([]);
         |            ^
      70 |   });
      71 |   it('should filter channels based on the search text', () => {
      72 |     wrapper.setData({ loading: false, search: searchWord });

      at Object.<anonymous> (contentcuration/contentcuration/frontend/channelList/views/ChannelSet/__tests__/channelSelectionList.spec.js:69:12)
          at runMicrotasks (<anonymous>)

FAIL contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js
  ● Test suite failed to run

    Call retries were exceeded

      at ChildProcessWorker.initialize (node_modules/jest-worker/build/workers/ChildProcessWorker.js:193:21)

EshaanAgg · 2024-03-13T03:41:16Z

Thanks a lot for the help with debugging, @bjester! I'll try to make the relevant changes so that the same works! Thanks.

EshaanAgg · 2024-03-13T19:29:56Z

I have worked on fixing most of the tets. When running the tests locally, only 2 tests fail, and I am a bit confused as to how I might go about fixing the same:-

Answers Editor Component

studio/contentcuration/contentcuration/frontend/channelEdit/components/AnswersEditor/AnswersEditor.vue

Lines 46 to 52 in 5c0d00a

    
           <Checkbox 
        
             v-if="isMultipleSelection" 
        
             :key="answerIdx" 
        
             :value="answerIdx" 
        
             :input-value="correctAnswersIndices" 
        
             @change="onCorrectAnswersIndicesUpdate" 
        
           />

This component uses the input-value prop to set the state of the checkbox and then uses a custom on-change event to update the state. I am a bit unsure as to how I might go about supporting this functionality in the created wrapper.

Content Defaults Component

studio/contentcuration/contentcuration/frontend/shared/views/form/ContentDefaults.vue

Lines 74 to 85 in 5c0d00a

    
           <Checkbox 
        
             v-model="autoDeriveVideoThumbnail" 
        
             class="mt-2" 
        
             data-name="autoDeriveVideoThumbnail" 
        
             :label="$tr('videos')" 
        
           /> 
        
           <Checkbox 
        
             v-model="autoDeriveAudioThumbnail" 
        
             class="mt-2" 
        
             data-name="autoDeriveAudioThumbnail" 
        
             :label="translateConstant('audio')" 
        
           />

studio/contentcuration/contentcuration/frontend/shared/views/form/ContentDefaults.vue

Lines 178 to 187 in 5c0d00a

    
               computed: { 
        
                 author: generateGetterSetter('author'), 
        
                 provider: generateGetterSetter('provider'), 
        
                 aggregator: generateGetterSetter('aggregator'), 
        
                 copyrightHolder: generateGetterSetter('copyrightHolder'), 
        
                 licenseDescription: generateGetterSetter('licenseDescription'), 
        
                 autoDeriveAudioThumbnail: generateGetterSetter('autoDeriveAudioThumbnail'), 
        
                 autoDeriveDocumentThumbnail: generateGetterSetter('autoDeriveDocumentThumbnail'), 
        
                 autoDeriveHtml5Thumbnail: generateGetterSetter('autoDeriveHtml5Thumbnail'), 
        
                 autoDeriveVideoThumbnail: generateGetterSetter('autoDeriveVideoThumbnail'),

This component uses the v-model as an object with a get and set function. Though I have added support for the same and a validator of this object, I am having trouble implementing the functionality. Do you have any ideas of how we might support the same?

LianaHarris360 · 2024-03-15T19:16:26Z

For the issue with the Answers Editor component, the state prop is set to false when it should be an array. Updating line 50 in AnswersEditor.vue to use v-model instead of :input-value will properly connect the array of correct answer indices to the state prop. The other connected issue I noticed is that deselecting and selecting answers does not update the parent component of AnswersEditor correctly, if the question can have multiple answers.

EshaanAgg · 2024-03-17T19:57:35Z

For the issue with the Answers Editor component, the state prop is set to false when it should be an array. Updating line 50 in AnswersEditor.vue to use v-model instead of :input-value will properly connect the array of correct answer indices to the state prop. The other connected issue I noticed is that deselecting and selecting answers does not update the parent component of AnswersEditor correctly, if the question can have multiple answers.

Got it. It seem's to be that some logic surrounding all of these interactions and changes needs to be rewritten a bit. Will try to do so ASAP. Thanks so much for your time and help! I really appreciate it!

akolson

Hi @EshaanAgg. I think the changes made look great. Thanks.

Concerning the failing tests, I think that changing this to the correct prop should fix the problem for tests failing in contentDefaults.spec.js. Hopefully this helps.

EshaanAgg · 2024-03-18T20:26:52Z

@akolson Thanks for the help. I have updated the test suite in the ContentDefaults component to make use of the suggestion you provided, and it passes all the test suite now! I tried tackling the other two components again today, but am struggling with the implementation and figuring out a way to get the desired reactivity with the way the getter-setter based v-models. Will try taking out some more time to fix it.

akolson · 2024-03-19T14:51:24Z

Hi @EshaanAgg!

I think for your implementation of the Checkbox component to be compatible with the current code, It should also accept the input-value (or inputValue) prop. input-value is a Vuetify(v2) inspired prop that according to the documentation is The v-model bound value of the checkbox. I believe this value is synonymous to the the checked prop in the KCheckbox component in KDS. The difference is that checked holds the truthy state value of the checkbox, while input-value holds the checkbox's state value which could be for example an Array, boolean, or object (there could be others, didn't do a lot of auditing). We could say the the input-value is v-modelesque in a way, so I think setting the input-value of the Checkbox should also set the v-model prop of the KCheckbox.

Having the Checkbox accept both v-model and input-value is a way for us to keep backward compatibility until we are able to dedicate time to cleanup the usage variations. So unless the scope of this pr involves the cleanup, I think it would make more sense to have the component backward compatible. @bjester any varying thoughts or corrections?

bjester · 2024-03-20T14:57:48Z

@akolson That all sounds accurate to me.

@EshaanAgg There are some more linting issues

EshaanAgg · 2024-03-23T08:48:03Z

Thanks @akolson @bjester. I am taking a few days off due to some health reasons, but will definitely update the PR as soon as possible. Thanks again for taking out the time to leave such detailed comments and for all the help! I really appreciate the same.

This reverts commit dc3597e.

This reverts commit 51f74fd.

EshaanAgg · 2024-03-26T18:44:51Z

Hi! Thanks for all your patience! I have updated the PR such that the test suite passes completely locally and addressed all the linting issues. Hopefully, this PR is ready for review. Thanks again.

akolson · 2024-03-27T14:38:08Z

HI @EshaanAgg! Great work on resolving the failing tests. However, it looks like we have one more failing test in contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js I suspect that its related to the rules and hide-details props that have been removed from the Checkbox.vue component and yet used in the Create.vue component.

......
<Checkbox
  v-model="acceptedAgreement"
  :label="$tr('agreement')"
  required
  :rules="tosAndPolicyRules"
  :hide-details="false"
  class="my-1 policy-checkbox"
/>
......

I am unsure about the decision that was taken on these props apart from the discussion here, so some clarification could help resolve the issue. @bjester?

EshaanAgg · 2024-03-28T14:58:42Z

Thanks @akolson. I guess that the best way to handle the same would be to provide support for these props in the Checkbox component itself, as these give seemingly nice-to-have functionality that we may want to use in other UIs across Studio in the future (as these functionalities aren't specific to the Create component). If the same sounds good, I can work on implementing them today!

Thoughts @akolson @bjester?

EshaanAgg · 2024-03-29T18:57:02Z

Hey! I had completed adding support for the hideDetails and rules prop to our Checkbox component when I realized that we are using the VForm component's internal method .validate() while submitting the same.

studio/contentcuration/contentcuration/frontend/accounts/pages/Create.vue

Line 427 in fb765d2

if (this.$refs.form.validate()) {

Since this method relies on all the children components to be Vuetify components, I guess using KCheckbox here won't be feasible (unless we refactor the whole page). Just IMO, the best approach right now we can take for this page is to make use of the VCheckbox component here directly instead of the Checkbox wrapper so that we don't have to go into full-fledge refactoring, and we can continue the migration as well.

I have made this refactoring, removed the props hideDeatils and rules from our wrapper for now (as we do not require them, and there is no point in adding additional complexity to the component that we don't plan to use), and updated the PR with the same!

akolson · 2024-03-29T19:39:25Z

Thanks @EshaanAgg for raising your observations and for your dedication to the issue. We'll create a follow up issue to resolve the failure in contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js.

We'll approve and merge this issue for now.

Follow up issue

akolson

Approved!

akolson · 2024-03-29T20:01:01Z

contentcuration/contentcuration/frontend/accounts/pages/Create.vue

@@ -132,7 +132,7 @@
        </VSlideYTransition>

        <!-- Agreements -->
-        <Checkbox
+        <VCheckbox


This should be changed to Checkbox in #4492. Its only a temporal fix

bjester · 2024-03-29T23:25:06Z

@EshaanAgg Thank you for your contributions!

EshaanAgg added 2 commits March 9, 2024 01:06

introduce v-model approach

5a930e9

fix: remove un-necessary console.log

4ba2bdc

MisRob requested review from akolson, LianaHarris360, AlexVelezLl and ozer550 March 12, 2024 11:36

bjester self-assigned this Mar 12, 2024

EshaanAgg added 2 commits March 13, 2024 13:23

fix some tests

012aa27

fix more tests

f54a1d2

fix linting errors

4a09e3e

akolson requested changes Mar 18, 2024

View reviewed changes

fix another one

0c76f4c

EshaanAgg added 6 commits March 26, 2024 23:54

fix all tests

dc3597e

Revert "fix all tests"

51f74fd

This reverts commit dc3597e.

Revert "Revert "fix all tests""

d97c203

This reverts commit 51f74fd.

remove package-lock.json

e2c21bb

revert changes in yarn.lock

fd70391

Merge https://github.com/learningequality/studio into mig

8ae3793

add support for errors to checkbox

d6d9738

EshaanAgg added 2 commits March 30, 2024 00:31

fix create

e793fee

fix linting

d61ed39

akolson mentioned this pull request Mar 29, 2024

Create.vue needs migrating to KCheckbox #4492

Closed

akolson approved these changes Mar 29, 2024

View reviewed changes

akolson reviewed Mar 29, 2024

View reviewed changes

akolson merged commit c4d590d into learningequality:unstable Mar 29, 2024
13 checks passed

EshaanAgg mentioned this pull request Apr 1, 2024

Replace all checkboxes with KCheckbox in Studio learningequality/kolibri-design-system#254

Open

MisRob mentioned this pull request Apr 16, 2024

Added missing checkbox label #4512

Merged

24 tasks

MisRob mentioned this pull request Apr 23, 2024

Fixed the missing checkbox labels #4527

Closed

24 tasks

akolson mentioned this pull request Aug 13, 2024

Bulk Edit Release #4635

Merged

akolson mentioned this pull request Oct 1, 2024

Release v2024.10.02 #4776

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refac: change `Checkbox` to be a wrapper around `KCheckbox` instead of `VCheckbox` #4472

refac: change `Checkbox` to be a wrapper around `KCheckbox` instead of `VCheckbox` #4472

EshaanAgg commented Mar 8, 2024

MisRob commented Mar 12, 2024

bjester commented Mar 12, 2024 •

edited

Loading

EshaanAgg commented Mar 13, 2024

EshaanAgg commented Mar 13, 2024 •

edited

Loading

LianaHarris360 commented Mar 15, 2024

EshaanAgg commented Mar 17, 2024

akolson left a comment

EshaanAgg commented Mar 18, 2024

akolson commented Mar 19, 2024

bjester commented Mar 20, 2024

EshaanAgg commented Mar 23, 2024

EshaanAgg commented Mar 26, 2024

akolson commented Mar 27, 2024

EshaanAgg commented Mar 28, 2024

EshaanAgg commented Mar 29, 2024 •

edited

Loading

akolson commented Mar 29, 2024 •

edited

Loading

akolson left a comment

akolson Mar 29, 2024 •

edited

Loading

bjester commented Mar 29, 2024

refac: change Checkbox to be a wrapper around KCheckbox instead of VCheckbox #4472

refac: change Checkbox to be a wrapper around KCheckbox instead of VCheckbox #4472

Conversation

EshaanAgg commented Mar 8, 2024

Summary

Description of the change(s) you made

Manual verification steps performed

Does this introduce any tech-debt items?

Reviewer guidance

How can a reviewer test these changes?

Are there any risky areas that deserve extra testing?

Contributor's Checklist

Reviewer's Checklist

This section is for reviewers to fill out.

MisRob commented Mar 12, 2024

bjester commented Mar 12, 2024 • edited Loading

EshaanAgg commented Mar 13, 2024

EshaanAgg commented Mar 13, 2024 • edited Loading

LianaHarris360 commented Mar 15, 2024

EshaanAgg commented Mar 17, 2024

akolson left a comment

Choose a reason for hiding this comment

EshaanAgg commented Mar 18, 2024

akolson commented Mar 19, 2024

bjester commented Mar 20, 2024

EshaanAgg commented Mar 23, 2024

EshaanAgg commented Mar 26, 2024

akolson commented Mar 27, 2024

EshaanAgg commented Mar 28, 2024

EshaanAgg commented Mar 29, 2024 • edited Loading

akolson commented Mar 29, 2024 • edited Loading

akolson left a comment

Choose a reason for hiding this comment

akolson Mar 29, 2024 • edited Loading

Choose a reason for hiding this comment

bjester commented Mar 29, 2024

refac: change `Checkbox` to be a wrapper around `KCheckbox` instead of `VCheckbox` #4472

refac: change `Checkbox` to be a wrapper around `KCheckbox` instead of `VCheckbox` #4472

bjester commented Mar 12, 2024 •

edited

Loading

EshaanAgg commented Mar 13, 2024 •

edited

Loading

EshaanAgg commented Mar 29, 2024 •

edited

Loading

akolson commented Mar 29, 2024 •

edited

Loading

akolson Mar 29, 2024 •

edited

Loading