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

fix(list): performance issue in list items are now resolved #417

Merged
merged 8 commits into from
Nov 2, 2020

Conversation

YonatanKra
Copy link
Contributor

@github-actions
Copy link

github-actions bot commented Nov 1, 2020

PR showcase deployed here.

components/list/src/vwc-list.ts Outdated Show resolved Hide resolved
components/list/src/vwc-list.ts Outdated Show resolved Hide resolved
@YonatanKra YonatanKra requested a review from tveinfeld November 1, 2020 16:46
components/list/test/list-item.test.js Show resolved Hide resolved
components/list/src/vwc-list.ts Outdated Show resolved Hide resolved
components/list/src/vwc-list.ts Outdated Show resolved Hide resolved
components/list/test/list-item.test.js Show resolved Hide resolved
@YonatanKra YonatanKra force-pushed the mwc-list-performance-issue branch from 960e6d0 to afe1d69 Compare November 2, 2020 04:46
Avoids overhead in Safari + copes with the CRP 50ms task time
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 2, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@tveinfeld tveinfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that you're currently trying to work this out directly with Material. Do you think it's worth waiting for their response?

I'm concerned that applying Material bug patches to our code will lead to confusion, and might itself cause issues now or down the road when Material closes this.

Having said that: If the performance problem handled here is extremely grave and common, we might wish to apply it. What are you saying? How would you classify the severity here?

components/list/test/list-item.test.js Show resolved Hide resolved
components/list/test/list-item.test.js Show resolved Hide resolved
@YonatanKra YonatanKra requested a review from tveinfeld November 2, 2020 08:39
@YonatanKra
Copy link
Contributor Author

I understand that you're currently trying to work this out directly with Material. Do you think it's worth waiting for their response?

I'm concerned that applying Material bug patches to our code will lead to confusion, and might itself cause issues now or down the road when Material closes this.

Having said that: If the performance problem handled here is extremely grave and common, we might wish to apply it. What are you saying? How would you classify the severity here?

It is currently a blocker with Vonage.ai integartion, so yes. We should go with this one.
In addition, I currently have 3 open PRs that are ignored by mwc, so I don't count on them.

@YonatanKra YonatanKra changed the title fix: performance issue in list items are now resolved fix(list): performance issue in list items are now resolved Nov 2, 2020
@YonatanKra YonatanKra merged commit 824c715 into master Nov 2, 2020
@gullerya gullerya deleted the mwc-list-performance-issue branch November 23, 2020 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants