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

Add ID to upsell notice for better javascript call #6

Merged
merged 4 commits into from
Dec 13, 2020

Conversation

nicomollet
Copy link
Contributor

Hello

I created this PR to fix the upsell notice, as explained in https://wordpress.org/support/topic/admin-nag/

Thank you

@mahdiyazdani
Copy link
Member

@nicomollet Looks great, would you be able to have this updated to use CSS class name instead of ID?

@nicomollet
Copy link
Contributor Author

@mahdiyazdani I could, can you explain why the class is better than ID in this case? I suggested ID because it has a better javascript performance.

@mahdiyazdani
Copy link
Member

why the class is better than ID in this case?

I believe it depends on how the browser indexes the elements internally.
In my tests, the class name approach was faster on chrome and IE, but roughly the same on FF. What this means is that Chrome and IE index elements based on the class name and FF mostly doesn't or does a poor job at it.

However, it is not a big deal though, we can leave it as-is for now if you prefer.

I will have this merged into the next release. Thank you for your contribution.

@nicomollet
Copy link
Contributor Author

Thanks a lot @mahdiyazdani !

@mahdiyazdani mahdiyazdani merged commit a06509f into mypreview:master Dec 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants