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.

Sign up for GitHub

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

Jump to bottom

feat: instrument main via editable data attributes #143

Merged
merged 5 commits into from
Dec 1, 2022
Merged

feat: instrument main via editable data attributes #143

merged 5 commits into from
Dec 1, 2022

Conversation

mhaack
Copy link
Collaborator

@mhaack mhaack commented Nov 30, 2022 •
edited by meryllblanchet
Loading

@aem-code-sync
Copy link

aem-code-sync bot commented Nov 30, 2022 •
edited
Loading

Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

@aem-code-sync
Copy link

aem-code-sync bot commented Nov 30, 2022

Page Score PSI Audit Google
/drafts/mblanche/404.html Lighthouse returned error: ERRORED_DOCUMENT_REQUEST. Lighthouse was unable to reliably load the page you requested. Make sure you are testing the correct URL and that the server is properly responding to all requests. (Status code: 404) PSI
/drafts/mblanche/who-we-are SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-no-metadata SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-some-metadata SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link

aem-code-sync bot commented Nov 30, 2022

Page Score PSI Audit Google
/404.html SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-no-metadata SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-some-metadata SI FCP LCP TBT CLS PSI

@meryllblanchet meryllblanchet mentioned this pull request Nov 30, 2022
Closed
@aem-code-sync
Copy link

aem-code-sync bot commented Dec 1, 2022

Page Score PSI Audit Google
/404.html SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-no-metadata SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-some-metadata SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link

aem-code-sync bot commented Dec 1, 2022

Page Score PSI Audit Google
/404.html SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-no-metadata SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-some-metadata SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link

aem-code-sync bot commented Dec 1, 2022

Page Score PSI Audit Google
/404.html SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-no-metadata SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-some-metadata SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link

aem-code-sync bot commented Dec 1, 2022

Page Score PSI Audit Google
/404.html SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-no-metadata SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-some-metadata SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link

aem-code-sync bot commented Dec 1, 2022

Page Score PSI Audit Google
/404.html SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-no-metadata SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-some-metadata Lighthouse returned error: Something went wrong. PSI

@verlok-cn
Copy link

Hey @meryllblanchet, it looks like in the 404 page we have data-wp-page-cookie="blah", will it stay like that?
If yes, I'd rather remove the data-wp-page-cookie altogether.

verlok-cn
verlok-cn reviewed Dec 1, 2022
View reviewed changes
404.html Show resolved Hide resolved
verlok-cn
verlok-cn reviewed Dec 1, 2022
View reviewed changes
Copy link

@verlok-cn verlok-cn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • Please don't make the cookie value data-wp-page-cookie editable from editors
  • Please make the default value for cookies data-wp-page-cookie="not-accepted"

@verlok-cn
Copy link

Anyway I wouldn't block this merge, we can go ahead and change this in the future.

@meryllblanchet
Copy link
Contributor

Feature demoed to @verlok-cn and ready to go! cc @mhaack, feel free to merge.

@mhaack mhaack merged commit bc8a85a into main Dec 1, 2022
@mhaack mhaack deleted the 72-data-attributes-live-ux branch December 1, 2022 09:59
@aem-code-sync
Copy link

aem-code-sync bot commented Dec 1, 2022

Page Score PSI Audit Google
/404.html SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-no-metadata SI FCP LCP TBT CLS PSI
/drafts/mblanche/who-we-are-some-metadata SI FCP LCP TBT CLS PSI

@meryllblanchet meryllblanchet requested review from meryllblanchet and removed request for rofe December 1, 2022 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@verlok-cn verlok-cn verlok-cn left review comments

@meryllblanchet meryllblanchet Awaiting requested review from meryllblanchet

Assignees

@meryllblanchet meryllblanchet

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add data-attributes for LiveUX
3 participants
@mhaack @verlok-cn @meryllblanchet