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

bug: popover without ion-content not scrollable #29211

Closed
3 tasks done
capc0 opened this issue Mar 25, 2024 · 3 comments · Fixed by #29215
Closed
3 tasks done

bug: popover without ion-content not scrollable #29211

capc0 opened this issue Mar 25, 2024 · 3 comments · Fixed by #29215
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@capc0
Copy link
Contributor

capc0 commented Mar 25, 2024 •
edited
Loading

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

When a popup is created via PopoverController with a custom component, the contents of the popup is not scrollable.

image

Expected Behavior

The content of the popup should be scrollable (as it used to be with previous versions).

image

Steps to Reproduce

  1. Click on the button within the stackblitz
  2. Resize/shrink the windows viewport
  3. Oberserve that options at the end are cut off and it is not possible to scroll

Code Reproduction URL

https://stackblitz.com/edit/angular-vvbkmf?file=src%2Fapp%2Fapp.component.ts

Ionic Info

see Stackblitz setup

Additional Information

related to #28965 & #28963

a workaround is to set the following style in the globals:

ion-popover > * {
  overflow-y: auto;
}
@ionitron-bot ionitron-bot bot added the triage label Mar 25, 2024
@capc0 capc0 mentioned this issue Mar 25, 2024
Merged
2 tasks
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Mar 25, 2024 •
edited
Loading

Thanks for the report. The problem here is related to #28861. The code in the linked PR should have always applied, which means that this behavior should have never worked. Due to this bug the behavior noted here worked by chance.

I spoke with the team and we think it's worth trying to support this use case. We support modals without ion-content, so we should probably support the same for ion-popover too.

edit: Also note that this bug is specific to popovers to do not use ion-content.

@liamdebeasi liamdebeasi changed the title bug: popover with custom component not scrollable by default bug: popover without ion-content not scrollable Mar 25, 2024
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Mar 25, 2024
@ionitron-bot ionitron-bot bot removed the triage label Mar 25, 2024
@liamdebeasi liamdebeasi removed their assignment Mar 25, 2024
@liamdebeasi liamdebeasi mentioned this issue Mar 25, 2024
Merged
2 tasks
github-merge-queue bot pushed a commit that referenced this issue Apr 1, 2024
@liamdebeasi
fix(popover): viewport can be scrolled if no content present (#29215)
f08759c 
Issue number: resolves #29211

---------

<!-- 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. -->

In #28861 I fixed a
bug that caused `.popover-viewport` to have `overflow: hidden`. In
reality, this code should have always applied but due to an incorrect
selector it never did.

As it turns out in
#29211, some
developers were relying on the broken behavior to build their
applications. In particular, developers were using `ion-popover` without
an `ion-content`. The linked change made it so that using popovers
without `ion-content` were not scrollable.

After talking with @mapsandapps we think it makes sense to officially
support this behavior. We support using [modals without
`ion-content`](https://ionicframework.com/docs/api/modal#custom-dialogs),
and we could not think of a reason to not support the same use case for
popover.

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

- If the `.popover-viewport` element has a child content then
`.popover-viewport `will not be scrollable.
- If the `.popover-viewport` element does not have a child content then
`.popover-viewport` will be scrollable.

I implemented this behavior using progressive enhancement via `:has`.
The [`:has` pseudo-class](https://caniuse.com/?search=%3Ahas) has
cross-browser support. Ionic v7 supports some versions of browsers that
do not have `:has` support. As a result, we fall back to the existing
behavior in this environment. Developers are able to get this behavior
on older browsers by explicitly setting `overflow: auto` on
`.popover-viewport`.

Fortunately, we will be dropping support for several of the older
browsers versions in Ionic v8, so the need to do the manual opt-in
should be less frequent as time goes on.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## 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.8.2-dev.11711383079.118d48a5`

Testing:

1. Open https://codepen.io/liamdebeasi/pen/JjVJrZQ?editors=1100 (this
has a dev build installed)
2. Click each button to open a popover.
3. Verify that each popover can be scrolled.

I could not find a great way to automate this test, but let me know if
anyone has ideas!
@liamdebeasi
Copy link
Contributor

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

@ionitron-bot Ionitron Bot
Copy link

ionitron-bot bot commented May 1, 2024

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 May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(popover): viewport can be scrolled if no content present ionic-team/ionic-framework
3 participants
@liamdebeasi @brandyscarney @capc0