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

Add new select types to Select field #1301

Merged
merged 1 commit into from Jul 10, 2023

Conversation

EmmyMay
Copy link
Contributor

@EmmyMay EmmyMay commented Jul 5, 2023

The following changes are implemented

TODO: Summary

Changes in the user interface:

TODO: Add screenshots, recordings or remove this section

Checklist when submitting a final (!draft) PR

  • Commits are tidied up, squashed if needed and follow guidelines in CONTRIBUTING.md
  • Code builds
  • All existing tests pass
  • All new critical code is covered by tests
  • PR is linked to the relevant issue(s)
  • Rebased with the target branch

Closes #1272

@EmmyMay EmmyMay requested a review from Fajfa July 5, 2023 15:54
@EmmyMay EmmyMay self-assigned this Jul 5, 2023
<b-form-checkbox
v-if="f.options.selectType !== 'multiple'"
v-if="shouldAllowDuplicates"
Copy link
Member

Choose a reason for hiding this comment

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

This checkbox needs to be moved out of the form group, and same for User and Record selector

@@ -152,10 +151,11 @@ export default {
data () {
return {
newOption: { value: undefined, text: undefined, new: true },
selectOptions: [
selectOptionss: [
Copy link
Member

Choose a reason for hiding this comment

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

Two ss?

@@ -184,6 +184,23 @@ export default {
isNew () {
return this.module.moduleID === NoID || this.field.fieldID === NoID
},

selectOptions () {
const options = [
Copy link
Member

Choose a reason for hiding this comment

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

Why redefine it if you have it already defined in the data.

Instead of filtering like this, which then needs to be amended if a new option is added, do a property on the option object that will indicate if its only usable if multi. And then you can filter by that always.

},

shouldAllowDuplicates () {
return this.f.options.selectType !== 'multiple' && this.f.isMulti && this.f.options.selectType !== 'list'
Copy link
Member

Choose a reason for hiding this comment

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

Lets make this more readable, you can add a property to the option that will indicate if it supports duplicates.

if (!this.f.isMulti) return false

const { duplicates = false } = this.options.find(({ value }) => value === this.f.options.selectType) || {}
return duplicates 

@@ -92,6 +105,14 @@
</option>
</template>
</b-form-select>

<b-form-radio-group
v-else
Copy link
Member

Choose a reason for hiding this comment

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

Do it the other way around, if its list view display that, otherwise(v-else) the default.

@@ -30,7 +30,16 @@
</template>

<template v-if="field.isMulti">
<template v-if="field.options.selectType === 'list'">
Copy link
Member

Choose a reason for hiding this comment

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

Do we need an extra template?

@EmmyMay EmmyMay force-pushed the 2023.3.x-feature-select-list-view branch from 35933db to 58cc602 Compare July 7, 2023 11:33
@katrinDY
Copy link
Contributor

katrinDY commented Jul 7, 2023

LGTM

@EmmyMay EmmyMay force-pushed the 2023.3.x-feature-select-list-view branch from 58cc602 to a8e32a2 Compare July 10, 2023 14:08
@EmmyMay EmmyMay merged commit 75ef962 into 2023.3.x Jul 10, 2023
1 check passed
@EmmyMay EmmyMay deleted the 2023.3.x-feature-select-list-view branch July 10, 2023 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to display "Select" fields as Radio group/checkbox group for select fields
3 participants