Skip to content
This repository has been archived by the owner on Jun 22, 2022. It is now read-only.

Fix performance/regression caused by scoped _reset.scss + IE11 support #164

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

guvial
Copy link

@guvial guvial commented Feb 14, 2020

What kind of changes does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes:
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature

Other information:
It was a bad idea to scope the _reset.scss: performance is bad, lib becomes unusable with IE11 due to the fact that vue.js add css selectors based on attributes when it scopes css.
More information:
https://medium.com/@aantipov/ie11-performance-bottleneck-de304569361d
https://vue-loader.vuejs.org/guide/scoped-css.html

What I did:

  1. remove the scoped attribute of the _reset.scss imports

  2. remove all reset rules applied to based attributes (ex: body, div, button,...) to avoid that the global reset breaks other components on the same page (we use vue-atlas with other libs) and replace them by specific class selectors.
    In some cases when a reset rule seemed to be used only by specific components, I have tried to put this rules within the component instead of letting them in global scope.

This is now much more performant. It works very well in our apps but I have perhaps removed important global reset rules for other use cases I have not tested. In this case, I suggest not to put back these rules in global reset.scss to avoid breaking other non vue-atlas components, but to put them where they are required in VA components.

  1. fix an incoherence in VaInput component that was breaking IE11 compatibility. Both v-model and :value was defined for the same component instance. It breaks IE because it cannot handle object with 2 properties with same name ("value" in this case). I have remove the :value that did not seem to be useful.

  2. I have added few css rules specifically for IE11 in VaButton. It fixes alignment of icons in VaButton with IE11.

I think that this PR will also fix bug #159 .

Thanks in advance for your review and thanks again for the work on this great lib!

Enjoy your day!

Cheers

@guvial
Copy link
Author

guvial commented May 2, 2020

Hi folks,
This PR is open for a while and I would like to know if someone is still available to maintain the lib.
We like vue-atlas and we are ready to make it evolving.
We want to use vue-atlas more in the incoming weeks and we will add new features into our fork.
We would prefer to contribute and share our work broadly with other vue-atlas users.
If you need help to maintain this lib, validate the PR,... me and my colleague could help you.
Let me know if you are interested.
Enjoy your weekend, Cheers,
G

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants