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: svgs not getting rendered inside blocks #396

Merged
merged 1 commit into from
Nov 1, 2023
Merged

Conversation

vtsaplin
Copy link
Collaborator

@vtsaplin vtsaplin commented Oct 25, 2023

issue: #395

Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):

Fix #

Test URLs:

@aem-code-sync
Copy link

aem-code-sync bot commented Oct 25, 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

@vtsaplin vtsaplin requested a review from basecode October 25, 2023 15:08
@aem-code-sync
Copy link

aem-code-sync bot commented Oct 25, 2023

Page Scores Audits Google
/solutions/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Collaborator

@basecode basecode left a comment

Choose a reason for hiding this comment

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

👍

const ul = document.createElement('ul');
[...block.children].forEach((row) => {
const li = document.createElement('li');
li.innerHTML = row.innerHTML;
Array.from(row.children).forEach((child) => li.appendChild(child));
Copy link
Collaborator

@vradulescu-bd vradulescu-bd Oct 30, 2023

Choose a reason for hiding this comment

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

this is great, this solves the issue; so in conclusion we only need to call the decorateIcons() function just 1 time, right ( the the project level, in the scripts.js file ) ? also we have those remaining places that is also reproducing there, so they need fixes also ( I have updated https://svg-issue-fix--bitdefender--hlxsites.hlx.page/drafts/vlad/icons by adding the quotes carousel, and it's happening there ):
image

@vtsaplin vtsaplin merged commit 9bafcd9 into main Nov 1, 2023
2 of 3 checks passed
@vtsaplin vtsaplin deleted the svg-issue-fix branch November 1, 2023 13:09
@andreibogdan
Copy link
Collaborator

@ext-vradulescu-bf - I checked the link https://main--bitdefender--hlxsites.hlx.page/drafts/vlad/icons and could not spot any issue. Can you give me a hint for what I should look for?

image

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.

4 participants