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: bundle includes custom styles to support very old IE versions which cause UI issues #26464

Closed
4 of 7 tasks
ryaa opened this issue Dec 12, 2022 · 6 comments · Fixed by #26465
Closed
4 of 7 tasks
Labels
package: core @ionic/core package triage type: bug a confirmed bug report

Comments

@ryaa
Copy link

ryaa commented Dec 12, 2022

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x
  • Nightly

Current Behavior

The latest ionic 6.x does not support old IE browser versions - see https://ionicframework.com/docs/reference/browser-support, however, the bundle created still includes styles to fix some UI issues for old IE browser versions. For example, https://github.com/ionic-team/ionic-framework/blob/main/core/src/css/normalize.scss#L37

Please note that those styles are still included even though the .browserslistrc does not include the line to support for IE or Edge altogether. For example, here is my *.browserslistrc file

Chrome >=60
# Firefox >=63
# Edge >=79
Safari >=13
iOS >=13

The problem is that those fixes breaks some UI in my application (specifically, makes the images displayed with the wrong aspect ratio)

Expected Behavior

Not to include styles/fixes for unsupported/very old browsers

Steps to Reproduce

  1. create new ionic project from one of the started template projects
  2. add an image to the main page
  3. exclude support for IE/Edge browser from .browserslistrc
  4. build and run the application
  5. check that the image added has **max-width: 100%" style applied from normalize.css, which is the fix for IE 8/9/10 - see https://github.com/ionic-team/ionic-framework/blob/main/core/src/css/normalize.scss#L37

OR, you can clone this project https://github.com/ryaa/ionic-includes-old-ie-browser-styles, change to the project directory, run npm install and then ionic serve and check the styles applied to the image displayed on the main page (see the step 4 above).

Code Reproduction URL

https://github.com/ryaa/ionic-includes-old-ie-browser-styles

Ionic Info

Ionic:

   Ionic CLI                     : 6.20.3 (/Users/alexryltsov/.nvm/versions/node/v18.12.0/lib/node_modules/@ionic/cli)
   Ionic Framework               : @ionic/angular 6.4.0
   @angular-devkit/build-angular : 15.0.3
   @angular-devkit/schematics    : 15.0.3
   @angular/cli                  : 15.0.3
   @ionic/angular-toolkit        : 6.1.0

Capacitor:

   Capacitor CLI      : 4.6.1
   @capacitor/android : not installed
   @capacitor/core    : 4.6.1
   @capacitor/ios     : not installed

Utility:

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

System:

   NodeJS : v18.12.0 (/Users/alexryltsov/.nvm/versions/node/v18.12.0/bin/node)
   npm    : 9.0.0
   OS     : macOS Monterey

Additional Information

See the style from from normalize.css applied to the image
Screen Shot 2022-12-12 at 09 40 55
Screen Shot 2022-12-12 at 09 41 45

@ionitron-bot ionitron-bot bot added the triage label Dec 12, 2022
@liamdebeasi liamdebeasi self-assigned this Dec 12, 2022
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Dec 12, 2022
@liamdebeasi liamdebeasi removed their assignment Dec 12, 2022
@liamdebeasi
Copy link
Contributor

Thanks for the report. The border: 0 is for older versions of IE, not the max-width. The max-width makes it so that img does not flow outside of ion-content.

I do see that max-width is applied to the image in your example, but I do not see any visible differences when removing max-width. Do you have a screenshot of the incorrect behavior?

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Dec 12, 2022
@ryaa
Copy link
Author

ryaa commented Dec 12, 2022

I do see that max-width is applied to the image in your example, but I do not see any visible differences when removing max-width. Do you have a screenshot of the incorrect behavior?

In my case, this breaks the map ArcGIS layer displayed, it is not visible because its images have 0 width (this is due to the max-width: 100%; applied to the image which is rendered a child element of the leaflet canvas with width/height equal to 0)

When I apply max-width: none; to the images everything starts to work.

Please let me know if you need more details.

P.S. I understand that this might need to be fixed on my end, but, why do we need those old IE fixes still being added (even if .browserslistrc says not to add IE support)?

Thank you for the quick feedback.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 12, 2022
@liamdebeasi
Copy link
Contributor

Thanks for the reply. Are you able to ensure .leaflet-marker-icon has a higher selector specificity that way it overrides max-width: 100%?

P.S. I understand that this might need to be fixed on my end, but, why do we need those old IE fixes still being added (even if .browserslistrc says not to add IE support)?

We do not need this anymore, so I am removing the IE-specific code in #26465. It looks like this was added years ago and never reevaluated when we dropped IE support.

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Dec 12, 2022
@ionitron-bot ionitron-bot bot removed the triage label Dec 12, 2022
@ryaa
Copy link
Author

ryaa commented Dec 12, 2022

Thanks for the reply. Are you able to ensure .leaflet-marker-icon has a higher selector specificity that way it overrides max-width: 100%?

yes. adding the below to my app wide scss fixed my problem

.leaflet-marker-icon {
    max-width: none;
}

We do not need this anymore, so I am removing the IE-specific code in #26465. It looks like this was added years ago and never reevaluated when we dropped IE support.

Awesome. Thank you very much for the great support!!!

P.S. if you don't mind, i will remove the screenshots from my previous post for the security reasons. thank you again

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Dec 12, 2022
@liamdebeasi
Copy link
Contributor

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

@ionitron-bot
Copy link

ionitron-bot bot commented Jan 12, 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 Jan 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package triage type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants