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

feat(VTextField, VSelect): add clearable mixin (#4144, #5722, #6751) #7441

Closed
wants to merge 6 commits into from

Conversation

bludnic
Copy link
Contributor

@bludnic bludnic commented Jun 9, 2019

Description

Add clearable mixin to customize reset value by using reset-value prop.

Tested with these components: VTextField, VTextarea, VSelect, VAutocomplete, VCombobox, VOverflowBtn.

Motivation and Context

Solves these issues: #4144, #5722, #6751. And maybe #2752.

How Has This Been Tested?

unit, visually

Markup:

<template>
  <v-container>
    <!-- VTextField -->
    <v-layout
      wrap
      class="w300"
    >
      <h3 class="headline">TextField</h3>
      <v-text-field
        v-model="text"
        clearable
        reset-value="reset"
      ></v-text-field>
    </v-layout>

    <v-divider></v-divider>

    <!-- VTextarea -->
    <v-layout
      wrap
      class="w300"
      mt-5
    >
      <h3 class="headline">VTextarea</h3>
      <v-textarea
        v-model="text"
        clearable
        reset-value="reset"
      ></v-textarea>
    </v-layout>

    <v-divider></v-divider>

    <!-- VSelect -->
    <v-layout
      wrap
      class="w300"
      mt-5
    >
      <h3 class="headline">VSelect</h3>
      <v-flex xs12>
        <v-select
          v-model="select"
          :items="items"
          clearable
          reset-value="Vue"
        ></v-select>
      </v-flex>
    </v-layout>

    <v-divider></v-divider>

    <!-- VSelect (multiple) -->
    <v-layout
      wrap
      class="w300"
      mt-5
    >
      <h3 class="headline">VSelect (multiple)</h3>
      <v-flex xs12>
        <v-select
          v-model="selectMultiple"
          :items="items"
          :reset-value="['Vue']"
          clearable
          multiple
        ></v-select>
      </v-flex>
    </v-layout>

    <v-divider></v-divider>

    <!-- VAutocomplete -->
    <v-layout
      wrap
      class="w300"
      mt-5
    >
      <h3 class="headline">VAutocomplete</h3>
      <v-flex xs12>
        <v-autocomplete
          v-model="select"
          :items="items"
          :search-input.sync="search"
          placeholder="Search"
          hide-details
          clearable
          :reset-value="'Vue'"
        ></v-autocomplete>
      </v-flex>

      <v-flex xs12>
        <v-alert
          type="info"
          value="true"
        >
          {{ select }}
        </v-alert>
      </v-flex>
    </v-layout>

    <v-divider></v-divider>

    <!-- VCombobox -->
    <v-layout
      wrap
      class="w300"
      mt-5
    >
      <h3 class="headline">VCombobox</h3>
      <v-flex xs12>
        <v-combobox
          v-model="select"
          :items="items"
          clearable
          reset-value="Vue"
          chips
          label="Select"
        ></v-combobox>
      </v-flex>

      <v-flex xs12>
        <v-alert
          type="info"
          value="true"
        >
          {{ select }}
        </v-alert>
      </v-flex>
    </v-layout>

    <v-divider></v-divider>

    <!-- VCombobox (multiple) -->
    <v-layout
      wrap
      class="w300"
      mt-5
    >
      <h3 class="headline">VCombobox (multiple)</h3>
      <v-flex xs12>
        <v-combobox
          v-model="selectMultiple"
          :items="items"
          label="Select"
          multiple
          clearable
          :reset-value="['Vue']"
          chips
          deletable-chips
        ></v-combobox>
      </v-flex>

      <v-flex xs12>
        <v-alert
          type="info"
          value="true"
        >
          {{ selectMultiple }}
        </v-alert>
      </v-flex>
    </v-layout>

    <v-divider></v-divider>

    <!-- VOverflowBtn -->
    <v-layout
      wrap
      class="w300"
      mt-5
    >
      <h3 class="headline">VOverflowBtn</h3>
      <v-flex xs12>
        <v-overflow-btn
          v-model="select"
          :items="items"
          clearable
          reset-value="Vue"
        ></v-overflow-btn>
      </v-flex>

      <v-flex xs12>
        <v-alert
          type="info"
          value="true"
        >
          {{ select }}
        </v-alert>
      </v-flex>
    </v-layout>

    <v-divider></v-divider>

    <!-- VOverflowBtn (multiple) -->
    <v-layout
      wrap
      class="w300"
      mt-5
    >
      <h3 class="headline">VOverflowBtn (multiple)</h3>
      <v-flex xs12>
        <v-overflow-btn
          v-model="selectMultiple"
          :items="items"
          multiple
          clearable
          :reset-value="['Vue']"
        ></v-overflow-btn>
      </v-flex>

      <v-flex xs12>
        <v-alert
          type="info"
          value="true"
        >
          {{ selectMultiple }}
        </v-alert>
      </v-flex>
    </v-layout>
  </v-container>
</template>

<script>
  import VTextField from '../src/components/VTextField/VTextField'
  import VTextarea from '../src/components/VTextarea/VTextarea'
  import VSelect from '../src/components/VSelect/VSelect'
  import VCombobox from '../src/components/VCombobox/VCombobox'
  import VAutocomplete from '../src/components/VAutocomplete/VAutocomplete'
  import VOverflowBtn from '../src/components/VOverflowBtn/VOverflowBtn'
  export default {
    components: { VTextField, VTextarea, VSelect, VCombobox, VAutocomplete, VOverflowBtn },
    data: () => ({
      text: '',
      search: '',
      select: '',
      selectMultiple: [],
      items: [
        'Programming',
        'Design',
        'Vue',
        'Vuetify',
      ],
    }),
  }
</script>

<style>
.w300 {
  width: 300px
}
</style>

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and breaking changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@codecov
Copy link

codecov bot commented Jun 9, 2019

Codecov Report

Merging #7441 into dev will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             dev    #7441      +/-   ##
=========================================
+ Coverage   85.8%   85.82%   +0.02%     
=========================================
  Files        336      337       +1     
  Lines       9099     9107       +8     
  Branches    2418     2422       +4     
=========================================
+ Hits        7807     7816       +9     
+ Misses      1204     1203       -1     
  Partials      88       88
Impacted Files Coverage Δ
packages/vuetify/src/components/VSelect/VSelect.ts 94.22% <100%> (+0.45%) ⬆️
packages/vuetify/src/mixins/clearable/index.ts 100% <100%> (ø)
...es/vuetify/src/components/VTextField/VTextField.ts 95.38% <100%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ec6f05...274cdc0. Read the comment docs.

Copy link
Member

@MajesticPotatoe MajesticPotatoe left a comment

Choose a reason for hiding this comment

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

  • add prop description to docs for reset-value

@johnleider
Copy link
Member

I'm going to have to put this on hold until #7385 makes it in, thank you for your patience!

@johnleider johnleider added the S: on hold The issue is on hold until further notice label Jun 10, 2019
@johnleider
Copy link
Member

Reviewing this it feels weird that it's possible to clear the field and still have the clear icon visible. Possibly do a check to see if the reset-value === this.internalValue?

@johnleider johnleider added T: feature A new feature and removed S: on hold The issue is on hold until further notice labels Jun 19, 2019
@johnleider johnleider added this to the v2.x.x milestone Jun 26, 2019
@johnleider johnleider added the S: on hold The issue is on hold until further notice label Jun 26, 2019
@jacekkarczmarczyk jacekkarczmarczyk added S: has merge conflicts The pending Pull Request has merge conflicts and removed S: on hold The issue is on hold until further notice labels Jul 28, 2019
@johnleider johnleider changed the base branch from next to dev August 1, 2019 17:55
@johnleider johnleider added C: VSelect VSelect C: VTextField VTextField labels Aug 1, 2019
@johnleider
Copy link
Member

I've changed base-branch to dev. Please resolve the merge conflicts (these existed on next as well).

@TravisBuddy
Copy link

TravisBuddy commented Aug 1, 2019

Hey @bludnic,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: b40806b0-b4a1-11e9-a5f8-ad46f5ea24b0

@bludnic
Copy link
Contributor Author

bludnic commented Aug 1, 2019

Did I understand correctly, the VChip inside the VCombobox has no longer a v-chip__close, so this test is useless?
a7415fc#diff-f4de2ac835b084bb1e85b600680a6f26R145-R169

UPD:

I fixed this by adding deletableChips prop. Also updated Markup for VCombobox (multiple).

@jacekkarczmarczyk jacekkarczmarczyk removed the S: has merge conflicts The pending Pull Request has merge conflicts label Aug 1, 2019
* Fix unit tests from eager change

* VCombobox: Add missing prop `deletableChips`

* Update snapshots
@bludnic
Copy link
Contributor Author

bludnic commented Aug 2, 2019

Reviewing this it feels weird that it's possible to clear the field and still have the clear icon visible. Possibly do a check to see if the reset-value === this.internalValue?

What if resetValue is an array or an object? Should we do deep comparison?

@johnleider
Copy link
Member

Yea, that may be required considering that they could technically provide anything.

@johnleider
Copy link
Member

This Pull Request is being closed due to inactivity.

If you have any additional questions, please reach out to us in our Discord community.

@johnleider johnleider closed this Sep 24, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: VSelect VSelect C: VTextField VTextField T: feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants