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(select): popover can be scrolled #28965

Merged
merged 2 commits into from
Feb 5, 2024
Merged

fix(select): popover can be scrolled #28965

merged 2 commits into from
Feb 5, 2024

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Feb 2, 2024

Issue number: resolves #28963


What is the current behavior?

In #28861 I fixed an issue that caused the wrong content inside of a popover to be scrollable. This CSS should have been applied, but it broke back when popover was converted to the Shadow DOM. Fixing this issue revealed a misconfiguration with the select-popover that caused the select-popover to no longer be scrollable.

What is the new behavior?

  • Content inside of ion-select-popover can now be scrolled

Note that I am considering this a bug fix instead of a regression. While scrolling used to work in select-popover, it only worked by chance. The .popover-viewport styles should have always applied to the select-popover, thus requiring the use of overflow-y: auto in select-popover.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build: 7.7.1-dev.11706893059.1bef4b38

@github-actions github-actions bot added the package: core @ionic/core package label Feb 2, 2024
@liamdebeasi liamdebeasi marked this pull request as ready for review February 2, 2024 17:24
@liamdebeasi liamdebeasi requested a review from a team as a code owner February 2, 2024 17:24
@liamdebeasi liamdebeasi requested review from brandyscarney and a team and removed request for a team February 2, 2024 17:24
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM, works as intended

@liamdebeasi liamdebeasi added this pull request to the merge queue Feb 5, 2024
Merged via the queue into main with commit 35ab6b4 Feb 5, 2024
46 checks passed
@liamdebeasi liamdebeasi deleted the FW-5946 branch February 5, 2024 19:53
@tgangso
Copy link

tgangso commented Mar 12, 2024

FYI. This did not resolve the issue for me when using a component in the popover. Example:

this.popover.create({
component: XPopoverComponent,

But it works when I add the following CSS to the XPopoverComponent CSS file.

:host {
overflow-y: auto;
}

@capc0
Copy link
Contributor

capc0 commented Mar 19, 2024

@liamdebeasi I can confirm @tgangso issue with version 7.7.5

A workaround is to set the following in a global style set:

ion-popover>* {
    overflow-y: auto;
}

Should I open another issue?

@thetaPC
Copy link
Contributor

thetaPC commented Mar 19, 2024

Thank you for bringing this to our attention!

Yes, please open a new issue with a minimal repro so we can investigate this.

@capc0
Copy link
Contributor

capc0 commented Mar 25, 2024

Thank you for bringing this to our attention!

Yes, please open a new issue with a minimal repro so we can investigate this.

fyi I have created the following issue to track this: #29211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: popover from select not scrollable
4 participants