Updated support for sorting of drop downs #265
Conversation
|
Screenshot of Drop down sorted in descending order with Int selected as value type |
|
Screenshot of Dropdown sorted Asc with value set to Int and sorting on description field. |
|
Screenshot of a dropdown thats values are a string and sorted by description. Notice the None staying on top |
|
Screenshot of radio button groups sorted desc and asc. |
chessbyte
changed the title
| defaultDropdownField = tempValues.shift(); | ||
| } else if(_.isNull(tempValues[FIRST_OPTION][VALUE])) { | ||
| defaultDropdownField = tempValues.shift(); | ||
| } |
There was a problem hiding this comment.
Should we just combine this else if and the if with an || since both lines 58 and 60 do the same thing?
There was a problem hiding this comment.
I thought about that when i reviewed my own code. I think I will try that but will put the or on a separate line
| dropDownValues.push(option); | ||
| } | ||
| const value = (field.data_type === 'integer' ? parseInt(option[0], 10) : option[0]); | ||
| const description = (parseInt(option[1], 10) ? parseInt(option[1], 10) : option[1]); |
There was a problem hiding this comment.
Is this description used only for sorting?
Because if not, this breaks on any description which starts with a number. "3 horses" will become just 3
There was a problem hiding this comment.
Description is only used for sorting the drop down list.
There was a problem hiding this comment.
Oh.. and maybe an _.isNaN?
Otherwise 0 will still get sorted last.. (because parseInt("0", 10) is 0 => falsy)
EDIT: you can also use Number.isNaN if you don't like lodash, it is shimmed by es6-shim so no browser compatibility issues
There was a problem hiding this comment.
ha, you beat me to it. I just added isNan in just now.
| it('should allow a numeric Description field to be sorted in a dropdown', () => { | ||
| const testDropDownDescription = { | ||
| values: [ | ||
| [0, '0'], |
There was a problem hiding this comment.
Also, can you change the keys and values to something different (from each other)?
It is impossible to tell from this spec if this really works or if it just sorts by value instead :).
There was a problem hiding this comment.
Oh, and also without an 11 or something like that, it doesn't actually tell us if those are sorted as numbers or as strings..
There was a problem hiding this comment.
Also, we need to set the data.data_type to integer which is part of the original problem that javascript was sorting 1, 11, and 2 as if they were strings.
There was a problem hiding this comment.
Actually that wasn't the problem this PR addresses. This deals with a use case where a user would decide to actually use numbers in the description field and then decide they want to sort the dropdown by the description.
There was a problem hiding this comment.
@eclarizio , I went ahead and updated one of the test cases to actually specify the data type. It wasn't directly related to this test but I added it in for clarity sake.
|
Checked commits chalettu/ui-components@6d6663d~...0873a72 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 |
|
Released in 1.1.7 (master only) |
Updated support for sorting of drop downs (cherry picked from commit fa8af25)
|
Backported to |
|
Awesome! Thanks @chalettu @eclarizio @himdel |
|
Released in 1.0.16 (gaprindashvili) |
@miq-bot add_label bug
@miq-bot add_label gaprindashvili/yes
This PR also addresses an issue where "None" option should default to staying at the top of a dropdown list of options.
Fix for BZ https://bugzilla.redhat.com/show_bug.cgi?id=1531677