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(select): Fix issues with form-select #1673

Merged
merged 1 commit into from Mar 31, 2018

Conversation

Projects
None yet
4 participants
@robcresswell
Contributor

robcresswell commented Mar 12, 2018

The form-input component currently has some arbitary restrictions around
the combinations of multiple, custom-select and selectSize that will be
applied. Previously I tried to address this by passing through the
selectSize, but this caused an issue (hiding the arrows) in custom
selects when the size was set to 0.

This patch cleans up the internal computed state of form-select to be
more relaxed about what user input is rendered in the final component

Fixes: #1658

Rob Cresswell
fix(select): Fix issues with form-select
The form-input component currently has some arbitary restrictions around
the combinations of multiple, custom-select and selectSize that will be
applied. Previously I tried to address this by passing through the
selectSize, but this caused an issue (hiding the arrows) in custom
selects when the size was set to 0.

This patch cleans up the internal computed state of form-select to be
more relaxed about what user input is rendered in the final component

Fixes: #1658
@codecov

This comment has been minimized.

codecov bot commented Mar 12, 2018

Codecov Report

Merging #1673 into dev will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             dev    #1673      +/-   ##
=========================================
- Coverage   60.8%   60.79%   -0.02%     
=========================================
  Files        154      154              
  Lines       2873     2872       -1     
  Branches     790      789       -1     
=========================================
- Hits        1747     1746       -1     
  Misses       806      806              
  Partials     320      320
Impacted Files Coverage Δ
src/components/form-select/form-select.js 63.15% <100%> (-1.85%) ⬇️

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 21ca8fa...a30df1b. Read the comment docs.

@pi0

This comment has been minimized.

Member

pi0 commented Mar 31, 2018

Sorry for the delay. Thanks :)

@pi0

pi0 approved these changes Mar 31, 2018

@pi0 pi0 merged commit e3336c5 into bootstrap-vue:dev Mar 31, 2018

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 60.8%)
Details
codecov/project Absolute coverage decreased by -0.01% but relative coverage increased by +39.19% compared to 21ca8fa
Details
deploy/netlify Deploy preview ready!
Details
@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Aug 30, 2018

It should be noted that firefox has (or had) issues when select size is set to 1.

@robcresswell robcresswell deleted the robcresswell:fix-1658 branch Aug 30, 2018

@pi0

This comment has been minimized.

Member

pi0 commented Aug 30, 2018

OMG @tmorehouse you are back 😍😍😍😍😍😍😍😍

@robcresswell

This comment has been minimized.

Contributor

robcresswell commented Aug 30, 2018

hahaha @pi0 I had the same reaction! (But quietly in my head)

@pi0

This comment has been minimized.

Member

pi0 commented Aug 30, 2018

Let's have a chat on slack tonight, Friends 😸

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Aug 30, 2018

I'll see if I can get my slack working... We are now using slack at work, and was able to only log into my work slack account.

@mosinve

This comment has been minimized.

Member

mosinve commented Aug 30, 2018

I'm in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment