Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.

Create SubclassCheckbox component#151

Merged
guergana merged 2 commits into
masterfrom
subclassCheckbox-component
Jan 15, 2021
Merged

Create SubclassCheckbox component#151
guergana merged 2 commits into
masterfrom
subclassCheckbox-component

Conversation

@guergana
Copy link
Copy Markdown
Contributor

@guergana guergana commented Jan 14, 2021

first commit: added translations
second commit: added component and test

Comment thread src/components/SubclassCheckbox.vue Outdated
return this.$store.getters.subclasses;
},
set( value: boolean ): void {
this.$store.dispatch( 'setSubclasses', value );
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is missing the info about the conditionIndex. In general, I think it would be the better idea if this component doesn't talk to the store at all and just has a value prop and emits an input event when the checkbox is toggled. Let's let QueryCondition deal with the talking to the store.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, true.. 😓 totally forgot about the index.

@guergana guergana force-pushed the subclassCheckbox-component branch from 85e8c53 to 93ed2a7 Compare January 14, 2021 16:20
Copy link
Copy Markdown
Collaborator

@micgro42 micgro42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there, I think. Just one prop missing

@@ -0,0 +1,37 @@
<template>
<div class="querybuilder__include-subclasses">
<input
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we need a way to to actually set the checked state from the outside. I.e. some boolean prop (e.g. value or isChecked) that then is assigned to the checked attributed of the input 🙂 .

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean for when it's coming from the store at the start?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha.

@guergana guergana force-pushed the subclassCheckbox-component branch from 93ed2a7 to 7001c94 Compare January 15, 2021 12:20
props: {
isChecked: {
type: Boolean,
required: true,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will always be in the store. so i guess it makes sense to make it required.

@guergana guergana requested a review from micgro42 January 15, 2021 14:13
@guergana guergana force-pushed the subclassCheckbox-component branch from 7001c94 to ad46605 Compare January 15, 2021 14:35
Comment thread src/components/QueryCondition.vue Outdated
<div class="query-condition__toggle-button-group">
<Button disabled="true" style="border-right: 1px darkgrey solid;">with</Button>
<Button disabled="true">without</Button>
<NegationToggle value="without" />
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was added accidentally :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh! yes.

@guergana guergana force-pushed the subclassCheckbox-component branch from ad46605 to 5e6a917 Compare January 15, 2021 15:00
@guergana guergana merged commit dd47953 into master Jan 15, 2021
@guergana guergana deleted the subclassCheckbox-component branch January 15, 2021 15:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants