Skip to content

Conversation

Hyperkid123
Copy link
Member

closes #179

Changes

  • added new prop pluckSingleValue for pf3 select
  • you can now use array as a value for single select. It will pick the value of first item in array
  • [2, 4, 5] will result into 2 as a value in form state
  • also its now possible to use an array as a select value
[{
  label: 'Foo'
  value: [2, 4, 5]
}]

@Hyperkid123 Hyperkid123 requested review from skateman and rvsia October 22, 2019 13:12
@skateman
Copy link
Member

@Hyperkid123 does this mean that I have to use the pluckSingleValue? What happens if I use it on a multiselect?

@Hyperkid123
Copy link
Member Author

@skateman pluckSingleValue is by default true and is ignored on multi select.

@@ -164,6 +170,7 @@ Select.defaultProps = {
},
loadingMessage: 'Loading...',
simpleValue: true,
pluckSingleValue: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

This could bring some incompatibility issue, if someone uses an actual array as a value for the select.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep i though of that. Check the rest of the code and tests 😉

Copy link
Contributor

@rvsia rvsia Oct 22, 2019

Choose a reason for hiding this comment

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

I thought that if someone has options like this right now:

[
  { value:  ['apple', 'pineapple'], label: 'sweet'},
  { value: ['chocolate','kitkat'], label: 'best'},
] 

Then he should have to set this prop to false and that (he will need to change the schema) will break the backwards compatibility. Am I right?

Copy link
Member Author

Choose a reason for hiding this comment

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

The example you posted would not work at this point because we were using === to select the value from options instead if isEqual. So nobody could use it successfully

@skateman
Copy link
Member

Okay, then I just have to try it 😉

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval


|prop name|type|description|
|---------|----|-----------|
|pluckSingleValue|`bool`|If a value is an array, component will pick its first item and set it as a new value. This will override the value in state from `[2, 4, 5]` to `2` for example.|
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be said that it is a true by default

@Hyperkid123 Hyperkid123 force-pushed the allow-array-value-select branch from 4a813bc to 32438b0 Compare October 24, 2019 07:53
@codecov-io
Copy link

codecov-io commented Oct 24, 2019

Codecov Report

Merging #180 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
+ Coverage   81.52%   81.53%   +0.01%     
==========================================
  Files          87       87              
  Lines        1380     1397      +17     
  Branches      333      336       +3     
==========================================
+ Hits         1125     1139      +14     
- Misses        211      214       +3     
  Partials       44       44
Impacted Files Coverage Δ
...3-component-mapper/src/form-fields/select/index.js 97.22% <100%> (+0.16%) ⬆️
...mponent-mapper/src/form-fields/component-mapper.js 83.33% <0%> (-16.67%) ⬇️
...mponent-mapper/src/form-fields/component-mapper.js 71.42% <0%> (-3.58%) ⬇️
...mponent-mapper/src/form-fields/component-mapper.js 0% <0%> (ø) ⬆️
...s/react-form-renderer/src/form-renderer/helpers.js 100% <0%> (ø) ⬆️
...-form-renderer/src/parsers/miq-parser/constants.js 100% <0%> (ø) ⬆️
...rm-renderer/src/demo-schemas/miq-schemas/output.js 100% <0%> (ø) ⬆️
...ackages/react-form-renderer/src/constants/index.js 100% <0%> (ø) ⬆️

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 a48389e...32438b0. Read the comment docs.

Copy link
Contributor

@rvsia rvsia left a comment

Choose a reason for hiding this comment

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

👍 thanks for the doc changes!

@rvsia rvsia merged commit a5c4a95 into master Oct 24, 2019
@Hyperkid123 Hyperkid123 deleted the allow-array-value-select branch October 24, 2019 08:42
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.

Multiselect accepts non-array initialValue, single select doesn't accept array
4 participants