Header bottom nav #90

hparwani2 · 2023-10-03T08:44:38Z

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):

Fixes #39

Test URLs:

Original: https://www.sunstar.com
Before: https://main--sunstar--hlxsites.hlx.live
After: https://header-bottom-nav--sunstar--hlxsites.hlx.live
If you are adding a new block or a variation to an existing block, please fill below:

Block library path: https://--sunstar--hlxsites.hlx.page/tools/sidekick/library.html?plugin=blocks&path=/sidekick/blocks/&index=0

…r-bottom-nav

aem-code-sync · 2023-10-03T08:44:41Z

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

aem-code-sync · 2023-10-03T08:44:55Z

Page	Scores	Audits	Google
/

scripts/nav-tree-utils.js

bosschaert

Since nav-tree-utils.js seems like a re-usable library, it would be good to add some unit tests for it...

blocks/header/header.css

bosschaert · 2023-10-03T09:25:31Z

blocks/header/header.js

@@ -281,22 +105,26 @@ export default async function decorate(block) {
  const navMeta = getMetadata('nav');
  const navPath = navMeta || (getLanguage() === 'en' ? '/_drafts/himanshu/nav' : `/${getLanguage()}/nav`);


'/_drafts/himanshu/nav' ? 😉

Remove it before merge

JiangLong2019 · 2023-10-03T09:44:56Z

Hey @hparwani2, I'd like to review this but not exactly sure which function you implemented. Is it about changing hero images by mouseover?

blocks/header/header.css

blocks/header/header.js

jindaliiita · 2023-10-03T13:33:13Z

blocks/header/header.js

@@ -281,22 +105,26 @@ export default async function decorate(block) {
  const navMeta = getMetadata('nav');
  const navPath = navMeta || (getLanguage() === 'en' ? '/_drafts/himanshu/nav' : `/${getLanguage()}/nav`);


Remove it before merge

scripts/nav-tree-utils.js

jindaliiita · 2023-10-03T16:28:23Z

blocks/header/header.css

+  position: unset;
+}
+
+/* stylelint-disable-next-line no-descending-specificity */


why eslint rule disable here ?

blocks/header/header.css

aem-code-sync · 2023-10-04T12:20:45Z

Page	Scores	Audits	Google
/

github-actions · 2023-10-04T12:23:08Z

🔸 4 visual differences detected

/career/yuya-yoshisue (main vs branch)
Expected an image 1280px by 3441px, received 1280px by 3981px. 3026854 pixels (ratio 0.60 of all image pixels) are different.
/sidekick/blocks/hero (main vs branch)
Expected an image 1441px by 4004px, received 1441px by 4052px.
/sidekick/blocks/carousel (main vs branch)
Expected an image 1280px by 1306px, received 1280px by 1354px. 146329 pixels (ratio 0.09 of all image pixels) are different.
/sidekick/blocks/career-apply (main vs branch)
40960 pixels (ratio 0.05 of all image pixels) are different.

The diff images are attached in the artifact

hparwani2 · 2023-10-04T13:21:27Z

Hey @hparwani2, I'd like to review this but not exactly sure which function you implemented. Is it about changing hero images by mouseover?

@JiangLong2019 I have created the bottom nav of the header or cateogoryTree of header. please refer the below screenshot.

sdmcraft · 2023-10-05T05:42:58Z

scripts/nav-tree-utils.js

@@ -0,0 +1,84 @@
+import { createOptimizedPicture } from './lib-franklin.js';


Please consider moving this file under blocks/header if that's the only place its used.

sdmcraft · 2023-10-05T05:44:29Z

blocks/header/header.css

+header .mega-container .left-content-container .main-item-summary a h2 .icon.angle-right {
+  background-image: url('../../icons/angle-white-70.svg');
+  height: 24px;
+  width: 15px;


1rem=default browser font size (usually 16px)

sdmcraft · 2023-10-05T05:52:16Z

scripts/nav-tree-utils.js

+      if (children) {
+        const picture = createOptimizedPicture(data.image, '', false, [{ width: '800' }]);
+        li = htmlToElement(`<li class="drop menu-level-${level}-item"> 
+          <a class="link" href=${data.link}>${data.category}</a>


The data.link here and everywhere should be governed by the page domain. For e.g. hlx.live should have links with hlx.live domain and similarly for preview and prod domains.

JiangLong2019 · 2023-10-06T03:19:35Z

When mouseovers the link and overlap menu shows up, the other area is marked gray in the original site.

The link does not open a new window in the original site.

hparwani2 added 4 commits September 19, 2023 12:03

adding top and middle nav

ae50e26

review changes

7c234af

Merge branch 'main' of https://github.com/hlxsites/sunstar into heade…

8e9b958

…r-bottom-nav

adding bottom nav - desktop version

61c50ca

hparwani2 requested review from bosschaert, sdmcraft and jindaliiita October 3, 2023 08:45

hparwani2 commented Oct 3, 2023

View reviewed changes

scripts/nav-tree-utils.js Show resolved Hide resolved

bosschaert reviewed Oct 3, 2023

View reviewed changes

jindaliiita reviewed Oct 3, 2023

View reviewed changes

review comments

37fdbda

hparwani2 requested review from bosschaert and jindaliiita October 4, 2023 13:21

sdmcraft reviewed Oct 5, 2023

View reviewed changes

sdmcraft added this to the Milestone-3 milestone Oct 5, 2023

sdmcraft modified the milestones: Milestone-3, Milestone-4, Milestone-5 Oct 9, 2023

hparwani2 closed this Oct 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Header bottom nav #90

Header bottom nav #90

hparwani2 commented Oct 3, 2023 •

edited

Loading

aem-code-sync bot commented Oct 3, 2023

aem-code-sync bot commented Oct 3, 2023

bosschaert left a comment

bosschaert Oct 3, 2023

jindaliiita Oct 3, 2023

JiangLong2019 commented Oct 3, 2023

jindaliiita Oct 3, 2023

jindaliiita Oct 3, 2023

aem-code-sync bot commented Oct 4, 2023

github-actions bot commented Oct 4, 2023

hparwani2 commented Oct 4, 2023

sdmcraft Oct 5, 2023

sdmcraft Oct 5, 2023

sdmcraft Oct 5, 2023

JiangLong2019 commented Oct 6, 2023

		@@ -281,22 +105,26 @@ export default async function decorate(block) {
		const navMeta = getMetadata('nav');
		const navPath = navMeta \|\| (getLanguage() === 'en' ? '/_drafts/himanshu/nav' : `/${getLanguage()}/nav`);

		@@ -0,0 +1,84 @@
		import { createOptimizedPicture } from './lib-franklin.js';

Header bottom nav #90

Header bottom nav #90

Conversation

hparwani2 commented Oct 3, 2023 • edited Loading

aem-code-sync bot commented Oct 3, 2023

aem-code-sync bot commented Oct 3, 2023

bosschaert left a comment

Choose a reason for hiding this comment

bosschaert Oct 3, 2023

Choose a reason for hiding this comment

jindaliiita Oct 3, 2023

Choose a reason for hiding this comment

JiangLong2019 commented Oct 3, 2023

jindaliiita Oct 3, 2023

Choose a reason for hiding this comment

jindaliiita Oct 3, 2023

Choose a reason for hiding this comment

aem-code-sync bot commented Oct 4, 2023

github-actions bot commented Oct 4, 2023

🔸 4 visual differences detected

hparwani2 commented Oct 4, 2023

sdmcraft Oct 5, 2023

Choose a reason for hiding this comment

sdmcraft Oct 5, 2023

Choose a reason for hiding this comment

sdmcraft Oct 5, 2023

Choose a reason for hiding this comment

JiangLong2019 commented Oct 6, 2023

hparwani2 commented Oct 3, 2023 •

edited

Loading