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.

Sign up for GitHub

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

Jump to bottom

fix(k8s): fix bake manifest selector #7249

Merged
merged 1 commit into from
Jul 23, 2019
Merged

fix(k8s): fix bake manifest selector #7249

merged 1 commit into from
Jul 23, 2019

Conversation

ethanfrogers
Copy link
Contributor

maggieneterval
maggieneterval approved these changes Jul 23, 2019
View reviewed changes
Copy link
Contributor

@maggieneterval maggieneterval left a comment

Choose a reason for hiding this comment

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

Good catch, Ethan! 💯

Only suggestion is to fix up the typescript definitions to match the true type. This will also allow us to remove the Option import from react-select on line 2.

app/scripts/modules/core/src/pipeline/config/stages/bakeManifest/BakeManifestStageForm.tsx Outdated
@@ -112,7 +112,7 @@ export class BakeManifestStageForm extends React.Component<
<ReactSelectInput
clearable={false}
onChange={(o: Option<string>) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
onChange={(o: Option<string>) => {
onChange={(e: React.ChangeEvent<HTMLSelectElement>) => {

@maggieneterval
Copy link
Contributor

@Jammy-Louie I did a quick search for the ReactSelectInput component elsewhere in the codebase and noticed a few other spots where I think we are mis-labelling React.ChangeEvent as Option types in the onChange handler -- Typescript isn't yelling at us for trying to read target.value off an Option because it can accept arbitrary key-value pairs. Just something to be aware of as we convert more forms to React!

@ethanfrogers ethanfrogers merged commit a61aa7a into spinnaker:master Jul 23, 2019
@ethanfrogers
Copy link
Contributor Author

@spinnakerbot cherry-pick 1.15

spinnakerbot pushed a commit that referenced this pull request Jul 23, 2019
@ethanfrogers
fix(k8s): fix bake manifest selector (#7249)
809b25f 
Fixes spinnaker/spinnaker#4656
@spinnakerbot
Copy link
Contributor

Cherry pick successful: #7250

@ethanfrogers ethanfrogers deleted the fix-bake-selector branch July 23, 2019 16:37
ethanfrogers pushed a commit that referenced this pull request Jul 23, 2019
@spinnakerbot @ethanfrogers
fix(k8s): fix bake manifest selector (#7249) (#7250)
4839aaa 
Fixes spinnaker/spinnaker#4656
christopherthielen added a commit to christopherthielen/deck that referenced this pull request Jul 26, 2019
@christopherthielen
chore(core): Bump version to 0.0.396
98b0b0a 
d73fc99 fix(core): Prevent reloads when hitting enter in create pipeline modal (spinnaker#7277)
96e1b08 feat(ui): add spinnaker version to UI, addresses spinnaker#4383 (spinnaker#7254)
592392f fix(core): remove the logic to initialize after props change (spinnaker#7271)
d60b3bc refactor(core/pipeline): Remove TriggerFieldLayout and BaseTrigger - Switch from TriggerFieldLayout to StandardFieldLayout - Switch to rendering Trigger description inside the dropdown
9c9faf4 fix(core/presentation): StandardFieldLayout: add top margin, flex first element, render blank label - Add margin-top between fields - Flex-fill the first element in the input content area - Render the label section even when passed an empty string - Add CSS classes to layout sections for customization by callers
f8c8f94 fix(core/presentation): Fix mount check -- useRef instead of useState
9c7885f feat(core/presentation): Add a <Formik/> wrapper which applies fixes and opinions that we want in Spinnaker (spinnaker#7272)
4adc3d4 feat(core/entityTag): Add maxResults to settings.js (spinnaker#7270)
608b83a refactor(core/details): Creating generic component for entity details (spinnaker#7262)
71a4d9e refactor(core): reactify notification and manual execution modal (spinnaker#7075)
bec74d9 refactor(core/pipeline): Refactor most trigger to use <FormField/> components (spinnaker#7255)
661e021 fix(core/pipeline): stage executionDetailsSections resolving the wrong cloudProvider (spinnaker#7260)
4dba1b1 fix(k8s/runJob): External logs URL to support manifest with implicit default namespace (spinnaker#7252)
2ca4eef fix(k8s): fix job log modal overflow (spinnaker#7256)
64fc418 feat(core/presentation): Add virtualized support to ReactSelectInput (spinnaker#7253)
c58b2bd fix(artifacts): correct bitbucket placeholder text (spinnaker#7251)
a61aa7a fix(k8s): fix bake manifest selector (spinnaker#7249)
4676096 fix(core/presentation): Default FormField 'name' prop to '', not noop (spinnaker#7248)
040c80f fix(core/pipeline): When changing trigger type, only retain the fields common to all trigger types (spinnaker#7247)
@christopherthielen christopherthielen mentioned this pull request Jul 26, 2019
Merged
christopherthielen added a commit that referenced this pull request Jul 26, 2019
@christopherthielen
chore(core): Bump version to 0.0.396
b07510e 
d73fc99 fix(core): Prevent reloads when hitting enter in create pipeline modal (#7277)
96e1b08 feat(ui): add spinnaker version to UI, addresses #4383 (#7254)
592392f fix(core): remove the logic to initialize after props change (#7271)
d60b3bc refactor(core/pipeline): Remove TriggerFieldLayout and BaseTrigger - Switch from TriggerFieldLayout to StandardFieldLayout - Switch to rendering Trigger description inside the dropdown
9c9faf4 fix(core/presentation): StandardFieldLayout: add top margin, flex first element, render blank label - Add margin-top between fields - Flex-fill the first element in the input content area - Render the label section even when passed an empty string - Add CSS classes to layout sections for customization by callers
f8c8f94 fix(core/presentation): Fix mount check -- useRef instead of useState
9c7885f feat(core/presentation): Add a <Formik/> wrapper which applies fixes and opinions that we want in Spinnaker (#7272)
4adc3d4 feat(core/entityTag): Add maxResults to settings.js (#7270)
608b83a refactor(core/details): Creating generic component for entity details (#7262)
71a4d9e refactor(core): reactify notification and manual execution modal (#7075)
bec74d9 refactor(core/pipeline): Refactor most trigger to use <FormField/> components (#7255)
661e021 fix(core/pipeline): stage executionDetailsSections resolving the wrong cloudProvider (#7260)
4dba1b1 fix(k8s/runJob): External logs URL to support manifest with implicit default namespace (#7252)
2ca4eef fix(k8s): fix job log modal overflow (#7256)
64fc418 feat(core/presentation): Add virtualized support to ReactSelectInput (#7253)
c58b2bd fix(artifacts): correct bitbucket placeholder text (#7251)
a61aa7a fix(k8s): fix bake manifest selector (#7249)
4676096 fix(core/presentation): Default FormField 'name' prop to '', not noop (#7248)
040c80f fix(core/pipeline): When changing trigger type, only retain the fields common to all trigger types (#7247)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maggieneterval maggieneterval maggieneterval approved these changes

@Jammy-Louie Jammy-Louie Awaiting requested review from Jammy-Louie

Assignees
No one assigned
Labels
target-release/1.16
Projects
None yet
Milestone
No milestone
3 participants
@ethanfrogers @maggieneterval @spinnakerbot