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.

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 advanced settings button name on create form #6667

Merged
merged 1 commit into from
Dec 20, 2021
Merged

Fix advanced settings button name on create form #6667

merged 1 commit into from
Dec 20, 2021

Conversation

prnam
Copy link

@prnam prnam commented Dec 19, 2021

This change will fix switch advanced settings button name present on Create from form

This what we see currently -
Action to hide is executing perfectly, however button name is always "Show advanced settings".

{isMoreOptionsEnabled(), select, 1 {Hide advanced options} other {Show advanced options}}

before-one

before-two

After PR is accepted this how it will look -

{isMoreOptionsEnabled(), select, true {Hide advanced options} other {Show advanced options}}

after-one

after-two

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 19, 2021
@k8s-ci-robot k8s-ci-robot added language/fr Updates or issues for French translations. language/ja Updates or issues for Japanese translations. language/ko Updates or issues for Korean translations. language/zh Updates or issues for Chinese translations. labels Dec 19, 2021
@floreks
Copy link
Member

floreks commented Dec 19, 2021

"Hide Advanced Options" is intentional. It indicates what will happen after clicking this button. We do not want to have 2x the same message. It should be dynamic.

@prnam
Copy link
Author

prnam commented Dec 19, 2021

"Hide Advanced Options" is intentional. It indicates what will happen after clicking this button. We do not want to have 2x the same message. It should be dynamic.

I did not understand you, @floreks. Did you mean you support this PR which will change current 2x same message ( i.e Show advanced settings) to getting Hide advanced settings text working on the button? 🤔

@prnam
Copy link
Author

prnam commented Dec 20, 2021

Hey @chenrui333, @helight,
Kindly review this PR and let me know if there is anything you need from my end. 🙂

@floreks
Copy link
Member

floreks commented Dec 20, 2021

"Hide Advanced Options" is intentional. It indicates what will happen after clicking this button. We do not want to have 2x the same message. It should be dynamic.

I did not understand you, @floreks. Did you mean you support this PR which will change current 2x same message ( i.e Show advanced settings) to getting Hide advanced settings text working on the button? 🤔

Sorry for the late response. Initial issue description was a bit misleading for me also and I thought we already have Show/Hide text.

This change LGTM :)

@floreks
Copy link
Member

floreks commented Dec 20, 2021

/lgtm

@floreks
Copy link
Member

floreks commented Dec 20, 2021

/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 20, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floreks, prnam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floreks, prnam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2021
@k8s-ci-robot k8s-ci-robot merged commit f455809 into kubernetes:master Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/de Updates or issues for German translations. language/fr Updates or issues for French translations. language/ja Updates or issues for Japanese translations. language/ko Updates or issues for Korean translations. language/zh Updates or issues for Chinese translations. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants