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

feat(fragment) - inherit section dataset #19

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

andreituicu
Copy link
Collaborator

@andreituicu andreituicu commented Nov 20, 2023

Building on @rofe's #4 where the fragment section's classes were propagated to the target section, this PR also propagates the dataset.
In the boilerplate, section metadata:

Test URLs:
Before: https://main--helix-block-collection--adobe.hlx.page/block-collection/fragment
After: (cannot add after url, as I don't have permissions to push a branch in this repo)

Copy link

aem-code-sync bot commented Nov 20, 2023

Hello, I'm the AEM Code Sync 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

@rofe
Copy link
Contributor

rofe commented Nov 20, 2023

@andreituicu

cannot add after url, as I don't have permissions to push a branch in this repo

Alternatively, you can provide a URL on your fork.

@andreituicu
Copy link
Collaborator Author

@rofe I can't seem to make the fork work on the fragment-data branch... after installing the aem-code-sync application. I can access https://main--helix-block-collection--andreituicu.hlx.live/block-collection/fragment , but https://fragment-data--helix-block-collection--andreituicu.hlx.live/block-collection/fragment still returns 404.

I can show this in action in the project I'm currently working on:

The sidebar from that page is a (auto-blocked) fragment.
In the fragment: https://main--servicenow--hlxsites.hlx.live/blogs/fragments/sidebar-common-fragment
We have the following section metadata
Screenshot 2023-11-21 at 11 31 44

Before:

  • The style class (sidebar) was being propagated to the destination section, because of your changes from feat(fragment): inherit section classes #4
  • The data-start-sidebar-at-section="1" was not being propagated to the destination section ❌

After:
https://main--servicenow--hlxsites.hlx.live/blogs/topics/ai-and-automation

  • The data-start-sidebar-at-section="1" is also propagated to the destination section ✅
Screenshot 2023-11-21 at 11 37 13

Is this helpful?

@rofe
Copy link
Contributor

rofe commented Nov 21, 2023

https://fragment-data--helix-block-collection--andreituicu.hlx.live/block-collection/fragment still returns 404.
Yes, this is a known issue: if you created the branch before adding the bot, it will not automatically be sync'ed. You can sync it by POSTing to /*: https://www.aem.live/docs/admin.html#tag/code/operation/updateCode

Copy link

aem-code-sync bot commented Nov 21, 2023

Page Scores Audits Google
/block-collection/fragment PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
/block-collection/fragment PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@rofe rofe requested a review from davidnuescheler November 21, 2023 19:13
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