[BUG] Using range-slider in the kirby-page makes the vertical scrollbar disappear.

tmb-jyskefinans-dk commented 1 year ago

[x] I have written a good issue

Describe the bug

When using the range slider in a kirby-page, and the vertical scroll bar is visible, it removes the .scroll-y class from the ion-content > main element. As a result, the default .inner-scroll style is applied, which sets overflow:hidden. This issue occurs when a page using the range slider has more vertical content, causing the click on the range indicator to flicker upon clicking.

Describe how to reproduce the bug

Do a clean install of angular, and kirby with the default configuration.
Modify the app.component.html with the following.

<kirby-app>
<kirby-page>
  <kirby-header title="Test" subtitle1="Some test">
  </kirby-header>
  <kirby-page-content>
    <kirby-range minLabel="Min label" maxLabel="Max label" [max]="100" [min]="1" [value]="25"></kirby-range>
    ... Additional data to make the window scrollbar visible.
  </kirby-page-content>
</kirby-page>
</kirby-app>

Run the application click the range slider
See error -->

Which Kirby version was used?

8.6.0

What was the expected behavior?

If the scrollbar is visible due to long content, it should not remove the scrollbar upon clicking the range slider. The overflow-y : auto style is applied to the scroll-y class of the main element in the ion-content, and clicking the range slider removes that class.

Please complete the following information:

OS: [e.g. Windows, MacOS, iOS] Windows
Browser [e.g. Chrome, Safari] Chrome & Edge not tested in Safari should be the same though.
Browser version [e.g. 22] Edge v114.0.1823.67 Chrome v114.0.5736.199

Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Verification

To make sure the bug is not intended behaviour; it should be verified by a member of team Kirby before moving on to implementation.

[ ] Make sure the NOT verified label has been removed by a member of team Kirby; do not proceed until this is done

Implementation

The contributor who wants to implement this issue should:

[ ] Make sure you have read: "Before you get coding".
[ ] Signal to others you are working on the issue by assigning yourself.
[ ] Create a branch from the develop branch following our branch naming convention.
[ ] Create a test that reproduces the bug following guidelines in: "The good: Test".
[ ] Write code to fix the bug

Review

Once the issue has been implemented and is ready for review:

[ ] Do a self-review.
[ ] Create a pull-request.

tspringborg commented 7 months ago

This is in ionic: https://github.com/ionic-team/ionic-framework/issues/25595

tspringborg commented 7 months ago

Here's how we solved it with an ugly hack.

public toggleIonicOverflowHack(enable: boolean) {
    const id = 'ionic-overflow-hack';
    let sheet = document.querySelector(`#${id}`);
    if (enable) {
      if (!sheet) {
        sheet = document.createElement('style');
        sheet.id = id;
        sheet.innerHTML = `
          ion-content::part(scroll) {
            overflow-y: auto !important;
          }
        `;
        document.body.appendChild(sheet);
      }
    } else if (sheet) {
      document.body.removeChild(sheet);
    }
  }

<kirby-range
        (mousedown)="this.toggleIonicOverflowHack(true)"
        (window:mouseup)="this.toggleIonicOverflowHack(false)"
        ...
      ></kirby-range>

tmb-jyskefinans-dk commented 7 months ago

Did something similar. the issue on ionic has been there a long time sadly...

kirbydesign / designsystem