This repository was archived by the owner on Jun 22, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 22
Fix performance/regression caused by scoped _reset.scss + IE11 support #164
Open
guvial
wants to merge
17
commits into
nvms:master
Choose a base branch
from
apwide:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…elector on classes to get acceptable performance with IE 11
… only on tag names
Catch error when VaModal try to remove DOM node already removed
Stop propagation of click event when collapsing
Hi folks, |
- removed DatePicker components + moment dependency - avoid exporting unused components
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of changes does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
If adding a new feature, the PR's description includes:
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:
remove the scoped attribute of the _reset.scss imports
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.
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.
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