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

bug: ios, next and prev buttons do not work when calendar grid is only partially visible #27913

Closed
3 tasks done
thomasmassmann opened this issue Aug 2, 2023 · 11 comments · Fixed by #27917
Closed
3 tasks done
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@thomasmassmann
Copy link

thomasmassmann commented Aug 2, 2023

Prerequisites

Ionic Framework Version

v6.x, v7.x

Current Behavior

The arrow buttons on ion-datetime do not work on iOS devices (web, simulator, real device) when the calendar grid is only partially visible (less than 3 rows of days).

Bringing the calendar into the viewport (3 rows or more), the arrow buttons work again.

Expected Behavior

The arrow buttons to switch between months work, even when only the arrow buttons are currently in visible to the user.

Steps to Reproduce

  1. Open https://docs-demo.ionic.io/component/datetime
  2. Open one of the calendar widgets.
  3. Ensure the calendar is fully visible.
  4. The arrow buttons allow changing the month.
  5. Ensure only 2 rows of the calendar days are visible.
  6. Try to change the month (more than 2, either direction is fine).
  7. With the first click, the month selection stays at the current month, but the days grid slides in the next/previous month).
  8. Any additional click on the arrow button has no effect.

Code Reproduction URL

https://github.com/ionic-team/docs-demo

Ionic Info

[WARN] You are not in an Ionic project directory. Project context may be
       missing.

Ionic:

   Ionic CLI : 7.1.1

Utility:

   cordova-res : not installed globally
   native-run  : not installed globally

System:

   NodeJS : v18.16.0
   npm    : 9.5.1
   OS     : macOS Unknown

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Aug 2, 2023
@liamdebeasi liamdebeasi self-assigned this Aug 2, 2023
@liamdebeasi
Copy link
Contributor

Thanks for the report. The problem here is we get the month that is at the center of the datetime (on both x and y axes):

const elementAtCenter = root.elementFromPoint(box.x + box.width / 2, box.y + box.height / 2);

If the center of the datetime is out of view, then this call will return null. It's possible for either only the top or the bottom part of datetime to be visible, so we should account for this.

@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Aug 2, 2023
@ionitron-bot ionitron-bot bot removed triage labels Aug 2, 2023
@liamdebeasi
Copy link
Contributor

Here is a dev build if you are interested in testing the proposed fix:

7.2.2-dev.11690987396.11ef134f

Install Example:

npm install @ionic/[email protected]

@thomasmassmann
Copy link
Author

Hi @liamdebeasi , thanks for taking a look at this. I tried the dev version in a real world project and it still shows the same error.

@liamdebeasi
Copy link
Contributor

Can you provide the sample project so I can test? I tested the dev build on the link provided in the original post, and it is now working as expected.

@liamdebeasi liamdebeasi removed their assignment Aug 2, 2023
@thomasmassmann
Copy link
Author

Here is an example: https://github.com/thomasmassmann/ionicCalendarIssue

It is now working as long as a bit of 1 row with dates is visible. Only having the calendar header visible still results in the error.

My real-world project has the calendar in a modal, and in there in an accordion. That is a different thing to solve now.

@liamdebeasi
Copy link
Contributor

Thanks! Here is a new dev build if you would like to test:

7.2.2-dev.11690996559.1019674a

@thomasmassmann
Copy link
Author

Amazing!

@thomasmassmann
Copy link
Author

Just to confirm, the latest release now also fixes the issues when an ion-datetime is embedded in an ion-accordion.

@liamdebeasi
Copy link
Contributor

Great! I am going to keep this open until the linked PR is merged.

@liamdebeasi liamdebeasi reopened this Aug 3, 2023
github-merge-queue bot pushed a commit that referenced this issue Aug 4, 2023
Issue number: resolves #27913

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

When determining what the changed month is, we grab the element at the
center of the datetime and then grab the nearest calendar month. This
works fine if the datetime is fully in view, but if the center point is
out of the viewport then this will return `null`. As a result, scrolling
in the datetime will break.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- We now check scroll position instead of querying for DOM elements at
coordinates. This allows the view to continue to update even if the
entire calendar body is outside the viewport.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.2.2-dev.11690996559.1019674a`
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #27917, and a fix will be available in an upcoming release of Ionic Framework.

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 3, 2023

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants