[RadioGroup] [HtmlSelect] options support className and disabled #2783

cmslewis · 2018-08-08T22:13:17Z

Fixes #2782

Checklist

Include tests

Changes proposed in this pull request:

🐛 Fix regression: <RadioGroup> passes custom className to its options
🐛 Fix regression: <HtmlSelect> passes custom className to its options

Reviewers should focus on:

Nothing in particular. Should be simple.

Screenshot

N/A

cmslewis · 2018-08-08T22:14:32Z

packages/core/src/components/html-select/htmlSelect.tsx

@@ -60,8 +60,9 @@ export class HTMLSelect extends React.PureComponent<IHTMLSelectProps> {
        );

        const optionChildren = options.map(option => {
-            const { value, label }: IOptionProps = typeof option === "object" ? option : { value: option };
-            return <option key={value} value={value} children={label || value} />;
+            const { className: optionClassName, value, label }: IOptionProps =


Had to mix up the variable name to avoid shadowing the className variable on L59.

blueprint-bot · 2018-08-08T22:20:48Z

Fix: passes custom className to options (+ test)

Preview: documentation | landing | table

giladgray

tests 😖

giladgray · 2018-08-09T19:00:48Z

packages/core/test/controls/radioGroupTests.tsx

+        const OPTIONS = [{ className: "apple", value: "a" }, { className: "banana", value: "b" }];
+        const group = mount(<RadioGroup onChange={emptyHandler} options={OPTIONS} selectedValue="a" />);
+        assert.strictEqual(findRadio(group, { value: "a" }).prop("className"), "apple");
+        assert.strictEqual(findRadio(group, { value: "b" }).prop("className"), "banana");


enzyme provides wrapper.hasClass(name): boolean, but this is ok as error messages will be different.

but do move this near the other options tests

giladgray · 2018-08-09T19:02:26Z

packages/core/test/html-select/htmlSelectTests.tsx

+        return;
+    };
+
+    it("renders options without CSS classes by default", () => {


i hate to say it, but... 🤷‍♂️ since you're adding a test suite here then might as well test that all the options props come out as expected, not just className. keep it simple, short and sweet.

giladgray · 2018-08-10T19:18:14Z

@cmslewis i've discovered that HTMLSelect also doesn't support disabled right now, so we should def test all the props.

blueprint-bot · 2018-08-10T19:35:33Z

test all options props

Preview: documentation | landing | table

blueprint-bot · 2018-08-10T19:37:37Z

remove className test

Preview: documentation | landing | table

* Fix tabs querySelector on jsdom (palantir#2761) * match event handler prop name with code and docs (palantir#2760) * matched icons with the original design (palantir#2780) * [Popover] captureDismiss=false by default (palantir#2776) * disable captureDismiss by default as it breaks links * fix tests * redirect old v1, v2 sites to new URLs (palantir#2773) * Remove `HACKHACK` (palantir#2790) BP requires `"@types/react": "^16.0.34"` and it is pinned by `yarn.lock`. For this version of typings, it is provided an overload, then the `HACKHACK` does not persist. ```ts function createElement<P>( type: SFC<P> | ComponentClass<P> | string, props?: Attributes & P, ...children: ReactNode[]): ReactElement<P>; ``` See issue: palantir#2785 * [RadioGroup] [HtmlSelect] options support className and disabled (palantir#2783) * Fix: <RadioGroup> passes custom className to options (+ test) * Fix: <HtmlSelect> passes custom className to options (+ test) * HTMLSelect supports disabled * test all options props * remove className test * [DatePicker] ❤️ (palantir#2789) * tests for DateUtils.getDateTime() * replace setStateWithValueIfUncontrolled with setState & updateValue helper refactor handlers to reduce let vars and clarify logic * refactor constructor into helper functions * merge disabled checks * fix test for initial state * massage imports * little bug fixes - ignoring next month change when null - correct day calculation - comments! * bump react-day-picker * areSameDay calls areSameMonth * reduce nesting * fix tests * Add Popover support for "auto-start" and "auto-end" (palantir#2772) * palantir#2770: Popover now accepts 'auto-start' and 'auto-end' positions * Update example * Update docs * Oops, undo unintended changes to example * [Button] Utils.isEmptyReactNode solves icon regression (palantir#2775) * Utils.isEmptyReactNode solves button regression * naming and test * rename in tests * Publish - @blueprintjs/core@3.3.0 * updated Sketch file (palantir#2813) * [TagInput] On paste, don't tag-ify text if no separator included (palantir#2804) * [TagInput] Leave a delimiter-less value in the input on paste * Update tests * Update docs * [DateRangePicker] all tests use enzyme (palantir#2793) * DRP tests use enzyme everywhere with a cool harness * name clash * fix tests in React 15 by using accessor to find latest element when needed * [new] Divider component (palantir#2854) * add Divider component * example * docs edits * replace all modifiers with borders * ignore coverage * remove fill from example * english is hard * [DatePicker] reuse existing components in caption (palantir#2792) * HTMLSelect: add iconProps, fix dark icon color * use HTMLSelect in caption massive reduction in styles * DatePickerNavbar renders prev/next Buttons use Button for another reduction in styles * cache month widths * DatePicker uses single handleMonthChange for all DayPicker events caption (month/year select) and navbar (prev/next buttons) now all use the same logic for changes! * add DPNavbar to DRP * fix & refactor caption & DP tests assertSelectedDays() helper in DP tests replaces getSelectedDays() * add $datepicker-padding variable * adjust paddings to use standard buttons in navbar * month icon won't exceed select bounds * dateinput tests * renames * replace borders with Divider elements also remove all negative margins * copyright, test helper * use Divider component * fix DRP tests * adjust month select icon position * margin only on caption * fix R15 tests * [DatePicker] time support: timePrecision & timePickerProps (palantir#2856) * add TimePicker props right in DatePicker! * getDateTime() to merge date and time * remove "none" from allowed timePrecision * add style for TP in DP * refactors to MomentDate and PrecisionSelect to support time * add PrecisionSelect to DP and DRP examples * import types, update styles (no divider) * revert some example changes * check null in caption * revert DRP example change * DateInput only renders DatePicker * deprecate DateTimePicker * fix dateinput test * tests for time! * strict boolean, no only * top margin on timepicker * refactors to reduce some complexity (palantir#2858) * [DateRangePicker] Shortcuts component and renderCalendars method (palantir#2859) * Shortcuts component and renderCalendars method to greatly simplify render() * bind handleNextState * Publish - @blueprintjs/icons@3.1.0 * remove dependencies section * [table] fix invisible table menu icon (palantir#2866) Fixes palantir#2865 * [Spinner] restore IE support (palantir#2868) * fix Spinner in IE by adding HTML wrapper tag for the animation * fix loading button spinner position * add tagName test * attempt to fix changing value on IE * added latest version (palantir#2862) * Publish - @blueprintjs/core@3.4.0 - @blueprintjs/table@3.1.1 * sketch file updated date * sandbox link in readme * remove quotes on $ns variable value (palantir#2881) * [Icon] render HTML element & tagName prop (palantir#2884) * Icon tagName prop and set `.bp3-icon-{name}` on element * refactor icon styles so svg is child - only render font glyph if the icon element is :empty * icon docs * color prop becomes fill attribute on svg, overrides css colors * fix tests * oops fix icon classes on non-icon elements (like callouts & buttons) * fix text ref (palantir#2888) * Skeleton: fix FF support! (palantir#2887) and refactor styles for simpler keyframes * [OverflowList] fix browser zoom behavior (palantir#2886) * fix OverflowList when zoomed * less magical number * [docs] better version tag styles (palantir#2889) * [docs] better version tag styles * remove obsolete flex override on tag icons * center docs-nav-buttons and key-combo * [Skeleton] Increase animation contrast (palantir#2885) * [Spinner] add additional child element to isolate spinner from parent (palantir#2890) * add spinner-animation element to isolate spinner from parent * comments about elements * Publish - @blueprintjs/core@3.5.0 * [Icon] revert to inline-block (palantir#2896) * clean up comments * take no chances * revert to inline-block and set block on svg instead of relying on flex child fixes all noted regressions * Publish - @blueprintjs/core@3.5.1 * Incorrect argument name (palantir#2898) Copied this over and realized that the argument should not be item but rather film. * [Suggest] Added selectedItem prop (palantir#2874) * Added selected item prop on the ISuggestProps def. * Init the Suggest state with the selected item prop. * Fixed Suggest support for controlled mode. * Fixed a tslint coma issue. * Made the state the only source of truth, added tests. * Added more tests, improved controlled mode support Now the Suggest does not update its underlying state in controlled mode, just like the EditableText component. * Added support for controlled empty selection. * prop docs * whitespace * [TagInput] Use $input-padding-horizontal when empty for consistency with <InputGroup> (palantir#2900) * Use -padding-horizontal in empty <TagInput> * Remove -empty class, use pure CSS approach * Add left-icon toggle to example * 🔧 switch to tree-sitter-typescript (palantir#2908) * switch to tree-sitter-typescript, move syntax pkgs to docs-data * update syntax tokens * 🔧 switch to circle-github-bot (palantir#2907) * add circle-github-bot * new preview script * restore cache in circle job * delete old scripts * [timepicker] Fix allowing to type time that exceeds time bounds (palantir#2795) * [DateRangePicker] support time selection (palantir#2895) * add time selection unit tests * add maybeRender placeholder * add wrapper div for calendars + time * add time precision to DRP example * reorder function * DateRangePicker now has time selection field * Updated DateRangePicker example to show time when precision is selected * Updated props to include timePickerProps and fixed/cleaned up tests for DateRangePicker * renamed timepicker classes appropiately, and code cleanup * updated momentTime to pass props * clicking on a shortcut doesn't change the time * removed only from describe in tests * removed unneeded CSS property and added test for making sure that time is preserved when un-selecting / reselecting date * Um/fix collapse animation (palantir#2911) * fixes collapse opening animation on first open * fix documentation * Add condensed property to HTML tables (palantir#2904) * Add condensed property to HTML tables * Move description to small instead of condensed * Deprecate small property on HTML tables * comment format * move collapse animation state docs onto the enum values, CLOSING_END -> CLOSING (palantir#2914) * bump sass-inline-svg (palantir#2915) * [docs] Modifiers & update DTP deprecation (palantir#2909) * add modifiers docs * update DTP deprecation notice * back to present tense since we're ready to ship * [Switch] fix switch styles variables for dark theme (palantir#2912) * [Select] Add resetOnQuery prop (palantir#2894) * [Select] Add resetActiveItemOnQuery prop * Switch prop name from resetActiveItemOnQuery to resetOnQuery and default prop to true * Match documentation to current functionality * Move default prop down to the lowest level, queryList

Fix: <RadioGroup> passes custom className to options (+ test)

a605df6

cmslewis requested a review from giladgray August 8, 2018 22:13

Fix: <HtmlSelect> passes custom className to options (+ test)

d57644d

cmslewis force-pushed the cl/2782-radio-classname branch from 8edcf13 to d57644d Compare August 8, 2018 22:15

cmslewis commented Aug 8, 2018

View reviewed changes

cmslewis changed the title ~~Fix: <RadioGroup> passes custom className to its options~~ [RadioGroup] [HtmlSelect] Fix: Pass custom className to options Aug 8, 2018

giladgray suggested changes Aug 9, 2018

View reviewed changes

Gilad Gray added 3 commits August 10, 2018 12:30

HTMLSelect supports disabled

354fc65

test all options props

7891c46

remove className test

0ee501f

giladgray approved these changes Aug 10, 2018

View reviewed changes

giladgray changed the title ~~[RadioGroup] [HtmlSelect] Fix: Pass custom className to options~~ [RadioGroup] [HtmlSelect] options support className and disabled Aug 10, 2018

giladgray merged commit fdb6be1 into develop Aug 10, 2018

giladgray deleted the cl/2782-radio-classname branch August 10, 2018 19:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RadioGroup] [HtmlSelect] options support className and disabled #2783

[RadioGroup] [HtmlSelect] options support className and disabled #2783

cmslewis commented Aug 8, 2018

cmslewis Aug 8, 2018

blueprint-bot commented Aug 8, 2018

giladgray left a comment

giladgray Aug 9, 2018

giladgray Aug 9, 2018

giladgray Aug 9, 2018

giladgray commented Aug 10, 2018

blueprint-bot commented Aug 10, 2018

blueprint-bot commented Aug 10, 2018

[RadioGroup] [HtmlSelect] options support className and disabled #2783

[RadioGroup] [HtmlSelect] options support className and disabled #2783

Conversation

cmslewis commented Aug 8, 2018

Fixes #2782

Checklist

Changes proposed in this pull request:

Reviewers should focus on:

Screenshot

cmslewis Aug 8, 2018

Choose a reason for hiding this comment

blueprint-bot commented Aug 8, 2018

Fix: passes custom className to options (+ test)

giladgray left a comment

Choose a reason for hiding this comment

giladgray Aug 9, 2018

Choose a reason for hiding this comment

giladgray Aug 9, 2018

Choose a reason for hiding this comment

giladgray Aug 9, 2018

Choose a reason for hiding this comment

giladgray commented Aug 10, 2018

blueprint-bot commented Aug 10, 2018

test all options props

blueprint-bot commented Aug 10, 2018

remove className test