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: close AM / PM element by selecting #1663

Merged

Conversation

Anyrob
Copy link
Contributor

@Anyrob Anyrob commented Jun 27, 2020

Close AM / PM element by selecting

fix: #1475

Changes proposed in this PR:

  • When selecting an AM/PM option, the code is sending the value update but not the signal to close. The solution for this was to modify in the "AmPmSelect" render an action of "blur" (same as close) for the moment it is clicked, that means, AM / PM has been selected.

@nexxtway/react-rainbow

@commit-lint
Copy link

commit-lint bot commented Jun 27, 2020

Contributors

@Anyrob

Commit-Lint commands

You can trigger Commit-Lint actions by commenting on this PR:

  • @Commit-Lint merge patch will merge dependabot PR on "patch" versions (X.X.Y - Y change)
  • @Commit-Lint merge minor will merge dependabot PR on "minor" versions (X.Y.Y - Y change)
  • @Commit-Lint merge major will merge dependabot PR on "major" versions (Y.Y.Y - Y change)
  • @Commit-Lint merge disable will desactivate merge dependabot PR
  • @Commit-Lint review will approve dependabot PR
  • @Commit-Lint stop review will stop approve dependabot PR

@CLAassistant
Copy link

CLAassistant commented Jun 27, 2020

CLA assistant check
All committers have signed the CLA.

@TahimiLeonBravo TahimiLeonBravo changed the title Close AM / PM element by selecting fix: close AM / PM element by selecting Jun 27, 2020
@TahimiLeonBravo
Copy link
Collaborator

@Anyrob Good Job!!
Review the CLA because it found an inconsistency with your Github account, you can review this

https://help.github.com/en/github/committing-changes-to-your-project/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user

@Anyrob Anyrob force-pushed the ampm-element-close-automatically branch from dd524cb to 566e998 Compare June 28, 2020 01:11
@Anyrob
Copy link
Contributor Author

Anyrob commented Jun 28, 2020

Hi @TahimiLeonBravo, I already check the CLA, but it wasn't updating so I made another commit, and this time it works! Please let me know if there's another thing I need to do (:

@Saul9201 Saul9201 linked an issue Jun 28, 2020 that may be closed by this pull request
12 tasks
@Saul9201 Saul9201 removed a link to an issue Jun 28, 2020
12 tasks
@Anyrob Anyrob force-pushed the ampm-element-close-automatically branch 2 times, most recently from 927b4f1 to b99d304 Compare June 28, 2020 20:43
@Anyrob Anyrob force-pushed the ampm-element-close-automatically branch from b99d304 to 84df8a1 Compare June 28, 2020 21:49
@Anyrob
Copy link
Contributor Author

Anyrob commented Jun 28, 2020

@TahimiLeonBravo I made the change suggested by @LeandroTorresSicilia 👍 I think that's all if there is anything else pending please let me know

@@ -49,8 +49,7 @@ export default class AmPmSelect extends PureComponent {
}

render() {
const { isFocused } = this.props;
const { tabIndex, onFocus, value } = this.props;
const { isFocused, tabIndex, onFocus, value, onClick } = this.props;

Choose a reason for hiding this comment

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

good job!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you (: 💯

@LeandroTorresSicilia LeandroTorresSicilia merged commit 5ea877d into nexxtway:master Jun 28, 2020
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.

fix: TimePicker am/pm select when clicking on the selected element
4 participants