ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.02k stars 13.51k forks source link

bug: Remove margin from select popover. #23971

Closed infacto closed 3 years ago

infacto commented 3 years ago

Prequisites

Ionic Framework Version

Current Behavior

The popover always scrolls a pixel which looks weird and untidy.

demo 👎

image

Why is margin-top and margin-bottom set to -1px?

Related source code.

Expected Behavior

Don't scroll. Set margin to 0 and in this example padding is also 0. But margin 0 will fix this issue.

demo 👍

Steps to Reproduce

<ion-list class="ion-no-padding">
  <ion-item-group>
    <ion-item>
      <ion-label>Select Option</ion-label>
      <ion-select placeholder="Please select." interface="popover">
        <ion-select-option> A </ion-select-option>
        <ion-select-option> B </ion-select-option>
        <ion-select-option> C </ion-select-option>
      </ion-select>
    </ion-item>
  </ion-item-group>
</ion-list>

Play with that. Then add following css to the global.scss.

.sc-ion-select-popover {
  padding: 0 !important;
  margin: 0 !important;
}

Enjoy.

Personally I prever ion-no-padding for lists. We could set this by default for the list. Just consider it for the select popover interface. But the primary issue is to fix the unnecessary scroll behavior (margin).

Ionic Info

Ionic:

   Ionic CLI                     : 6.16.1
   Ionic Framework               : @ionic/angular 5.8.1
   @angular-devkit/build-angular : 12.1.4
   @angular-devkit/schematics    : 12.1.4
   @angular/cli                  : 12.1.4
   @ionic/angular-toolkit        : 4.0.0

System:

   NodeJS : v14.15.1
   npm    : 6.14.8
   OS     : Windows 10
liamdebeasi commented 3 years ago

Thanks, I can reproduce this. Looks like these -1px margins go all the way back to Ionic v3: https://github.com/ionic-team/ionic-v3/blob/master/src/components/select/select.scss#L7-L15.

The original context is no longer there as I cannot find an associated PR or GitHub issue. My current guess is this was added to work around some old Chromium/WebKit/UIWebView bug.

I will look into removing it.

edit: One side effect is that the -1px margins add the rubber band effect on non-scrollable content on iOS. That being said, I am not sure non-scrollable popovers should rubber band. I need to investigate this a bit more to what native does.

edit 2: Confirmed by testing a native popover on iOS that non-scrollable popovers should not rubber band.

liamdebeasi commented 3 years ago

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic-framework/pull/23972 and a fix will be available in an upcoming release of Ionic Framework.

ionitron-bot[bot] commented 2 years ago

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.