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

fix(VSelect): propagate name attribute to hidden input #10327

Merged
merged 5 commits into from
Jan 27, 2020

Conversation

cyruscollier
Copy link
Contributor

@cyruscollier cyruscollier commented Jan 25, 2020

Description

Resolves Issue #10152, a regression introduced by merging #9598.

This fixes the missing name attribute on the inner hidden input element inside VSelect. Since name is now removed from the inner text input, it needed to be manually added to the hidden input from the VSelect attribute list.

Motivation and Context

For users that rely on the traditional form input name/value structure to validate and/or submit forms, it's essential that all Vuetify field components maintain this underlying behavior.

How Has This Been Tested?

This PR Includes unit tests on VSelect and VCombobox to confirm the bug and verify the fix. It also completes the related unit test for the VSelect hidden input value by properly testing that changing the selected item updates the input value.

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 features but makes 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 backwards compatible changes and next for non-backwards compatible 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)

Previously the "VSelect should update inner input element" test only asserted the initial state of
the input value attribute. Now it also asserts that changing the selected item will update the input
value.
Fixes missing name attribute on inner input element by passing it manually from component
attributes. Includes unit tests on VSelect and VCombobox to verify bugfix.

fix vuetifyjs#10152
@jacekkarczmarczyk jacekkarczmarczyk added C: VSelect VSelect T: bug Functionality that does not work as intended/expected labels Jan 25, 2020
@@ -319,4 +319,17 @@ describe('VCombobox.ts', () => {

expect(wrapper.vm.$attrs.autocomplete).toBe('on')
})

it('should pass the name attribute to the inner input element', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

The genHiddenInput method is never overwritten and this is already tested in v-select https://github.com/vuetifyjs/vuetify/pull/10327/files#diff-5b5fcac8ce361d1dfbffa8cdea0bca32R453.

packages/vuetify/src/components/VSelect/VSelect.ts Outdated Show resolved Hide resolved
@johnleider johnleider added this to the v2.2.x milestone Jan 27, 2020
@johnleider johnleider changed the title Resolves Issue #10152 fix(VSelect): propagate name attribute to hidden input Jan 27, 2020
@johnleider johnleider added T: regression Something that used to work but we broke and removed T: bug Functionality that does not work as intended/expected labels Jan 27, 2020
@johnleider johnleider self-assigned this Jan 27, 2020
@cyruscollier
Copy link
Contributor Author

Thanks, @johnleider. Edits have been implemented.

@johnleider johnleider merged commit f289b05 into vuetifyjs:master Jan 27, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: VSelect VSelect T: regression Something that used to work but we broke
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants