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(datetime): recalculate day column when month is changed #17815

Merged
merged 14 commits into from Mar 21, 2019

Conversation

Projects
None yet
5 participants
@liamdebeasi
Copy link
Member

commented Mar 18, 2019

Short description of what this resolves:

This PR is meant to combine the changes proposed by #17562 and #14434 and to clean up the code a bit. Additionally, this PR also introduces more tests

Changes proposed in this pull request:

  • Add onIonPickerColChange event to ion-picker-column to allow the datetime component to be aware of the need to recalculate day values
  • Add tests for ion-picker-column
  • Add generic test utilities for interaction and listening for evens.

Ionic Version:

Fixes:
#14233
#14732
#15452
#15794
#16733
#17060
#17510
#17521

liamdebeasi added some commits Mar 14, 2019

@liamdebeasi liamdebeasi marked this pull request as ready for review Mar 18, 2019

@liamdebeasi liamdebeasi changed the title Datetime col change fix(datetime): recalculate day column when month is changed Mar 19, 2019

@liamdebeasi liamdebeasi requested a review from brandyscarney Mar 19, 2019

@brandyscarney

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

When we go to merge this, can we add @KillerCodeMonkey and @olivercodes as co-authors if this is going to close their PRs? https://help.github.com/en/articles/creating-a-commit-with-multiple-authors

liamdebeasi added some commits Mar 20, 2019

liamdebeasi and others added some commits Mar 20, 2019

Update core/src/components/picker-column/test/standalone/index.html
Co-Authored-By: liamdebeasi <liamdebeasi@users.noreply.github.com>
@KillerCodeMonkey

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

@liamdebeasi @brandyscarney nice this topic is on its way 🚀

If i can support you here, let me know :)

@brandyscarney

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

@KillerCodeMonkey Thank you! 🙂 So we found a bug with this PR but we're scheduled to release today so we aren't going to get this in today's release, but we're going to do our normal release and then release a dev build with this PR in it so we can get some community testing. We'll post the dev release here and on some of the datetime issues once it's out.

@KillerCodeMonkey

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

@brandyscarney

ah okay. so if you need help to solve the bug or need someone to test it 😉 .
I am happy you have this on your agenda, now.

@@ -325,6 +325,7 @@ export class Datetime implements ComponentInterface {
text: this.cancelText,
role: 'cancel',
handler: () => {
this.updateDatetimeValue(this.value);

This comment has been minimized.

Copy link
@KillerCodeMonkey

KillerCodeMonkey Mar 20, 2019

Contributor

@liamdebeasi thats why i stored the changed value on an internal State in my PR (only as an info) :)

This comment has been minimized.

Copy link
@liamdebeasi

liamdebeasi Mar 20, 2019

Author Member

Makes sense! I'd just rather reset it than store two copies of data that could potentially differ

@KillerCodeMonkey

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

@liamdebeasi liamdebeasi merged commit 9273f97 into master Mar 21, 2019

2 checks passed

build Workflow: build
Details
screenshot Screenshot
Details
@KillerCodeMonkey

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

liamdebeasi added a commit that referenced this pull request Mar 21, 2019

fix(datetime): recalculate day column when month or year is changed (#…
…17815)

Co-Authored-By: KillerCodeMonkey <bengtler@gmail.com>
Co-Authored-By: olivercodes <boliver@linux.com>
Co-Authored-By: liamdebeasi <liamdebeasi@users.noreply.github.com>

liamdebeasi added a commit that referenced this pull request Mar 21, 2019

@liamdebeasi

This comment has been minimized.

Copy link
Member Author

commented Mar 21, 2019

I was missing a space when adding your name/email for co-authors so I'm going to revert and try it again 🙂

@KillerCodeMonkey

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

liamdebeasi added a commit that referenced this pull request Mar 21, 2019

@liamdebeasi liamdebeasi deleted the datetime-col-change branch Mar 21, 2019

@zhiwo

This comment has been minimized.

Copy link

commented Apr 19, 2019

Short description of what this resolves:

This PR is meant to combine the changes proposed by #17562 and #14434 and to clean up the code a bit. Additionally, this PR also introduces more tests

Changes proposed in this pull request:

  • Add onIonPickerColChange event to ion-picker-column to allow the datetime component to be aware of the need to recalculate day values
  • Add tests for ion-picker-column
  • Add generic test utilities for interaction and listening for evens.

Ionic Version:

Fixes:
#14233
#14732
#15452
#15794
#16733
#17060
#17510
#17521

How use in Ionic?

@liamdebeasi

This comment has been minimized.

Copy link
Member Author

commented Apr 19, 2019

Hi there,

Please visit the Ionic DateTime Documention for instructions on how to use this component.

Thanks!

@olivercodes

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

Thank you for the co-author marks @liamdebeasi @brandyscarney, much appreciated

Kiku-git added a commit to Kiku-git/ionic that referenced this pull request May 16, 2019

fix(datetime): recalculate day column when month or year is changed (i…
…onic-team#17815)

Co-Authored-By: KillerCodeMonkey<bengtler@gmail.com>
Co-Authored-By: olivercodes <boliver@linux.com>
Co-Authored-By: liamdebeasi <liamdebeasi@users.noreply.github.com>

Kiku-git added a commit to Kiku-git/ionic that referenced this pull request May 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.