feature/issue 26 header component #44

Auhseh · 2024-05-05T23:06:16Z

Related Issue

#26

Summary of Changes

Create Header component with Storybook and test casss
Handle CSS Module Scripts support for ESLint and Vite <> Storybook

TODOs

Create basic storybook file
Create basic test case
add site links (e.g. <a> tags) to navigation
resolve image assets for Storybook
use open props values in CSS / align with designs
Use raw plugin to import SVGs
flesh out test cases
- check for navigation + links
- check for logo
- check for mobile nav
Fix bugs with mobile nav
- overlay does not hide anything
- state gets wonky after a couple clicks
mobile-menu-bugs.mov
Vite <> Storybook integration
- better pattern matching for constructable stylesheets
- re-use greenwood plugins for transformation

Nice to have / separate issue

dotted "track" lines - is this part of the overall layout as opposed to just the header? Maybe incorporate it into create a shared layout for all pages #32 ?
Make this component light DOM based and have an interactive only mobile menu? - optimize header component for static content / Light DOM #48
WCC queryXXX patches - upstream feature / fixes tracking #37
get official logo from source design assets - update Greenwood logo with official logo from original source assets #47

netlify · 2024-05-05T23:06:50Z

✅ Deploy Preview for super-tapioca-5987ce ready!

Name	Link
🔨 Latest commit	`620f6c3`
🔍 Latest deploy log	https://app.netlify.com/sites/super-tapioca-5987ce/deploys/6647b4879673660008e9df22
😎 Deploy Preview	https://deploy-preview-44--super-tapioca-5987ce.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

thescientist13

@Auhseh
Thanks for getting this started. I think for now as I referenced in this PR let's please avoid adding multiple unrelated changes in the one PR. This PR ideally would just be focused on the header and thus probably should only have three files added

header.js
header.stories.js
header.spec.js

The storybook file in particular is of value to help develop and test the component in isolation.

I would also ask that we try and follow the contributing guidelines if possible, in particular having linting / formatting applied so we don't keep having issues with casing of filenames and formatting of files that are hard to read. Not sure if that's possible from your but it really helps on the review side of things.

Let me try and clean some things up here and get the focus on just the header component for now.

thescientist13 · 2024-05-12T22:37:56Z

src/pages/blog/blog-landing.html

@@ -0,0 +1,74 @@
+<!DOCTYPE html>


@Auhseh
Please revert any changes unrelated to the header component please, including any static assets. Those should be done as part of another issue / PR (which I plan to work on initially)

thescientist13 · 2024-05-12T22:39:23Z

src/styles/main.css

@@ -1,3 +1,23 @@
+@import "https://unpkg.com/open-props";


not sure if something is getting confused here, but open-props are already being defined in theme.css as well some of these global styles.

thescientist13 · 2024-05-12T22:39:48Z

src/styles/main.css

+/**-----------------------------------------------------------------**/
+
+
+/*-----BLOG STYLING--------------------------------------------------*/


see my comments above re: doing multiple tasks in one PR.

Yea, Sorry about this

thescientist13 · 2024-05-12T22:44:47Z

src/components/header/header.js

+
+                <div class="social-tray">
+                    <li class="social-icon">
+                        <svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">


If these are the SVGs that we already committed into the project, let me see if I can statically pull these in as text, otherwise it would be a shame to duplicate that content / code.

I'm also thinking that since only menu is interactive, that the header itself can probably be served statically and that we can create a mobile-menu component which can focus on just the interactive bits, let me try and play around with that.

thescientist13 · 2024-05-12T23:34:21Z

src/components/header/header.js

+
+        <header>
+            <div class="logo-bar">
+                <img class="greenwood-logo" src="/assets/greenwood-logo.svg" alt="Logo">


I notice this isn't the same logo from our old site, which I think was what you mentioned, so let me make a note of that and try and get the original source, though worst case scenario we can just re-use the images for now.

thescientist13 · 2024-05-12T23:37:40Z

src/components/header/header.js

+            }
+
+            header {
+                height: 5.5rem;


are we not able to leverage more open props values here instead of hardcoding our own odd values?

Maybe i missed it, but the issue seems to be that open-props doesn't have values for 5.5, 11rem etc. and i can't seem to figure out how to customize with these values we have in the design.

I will try and take a look, but i think generally most design systems work off of even values generally? What is closest to what we need vs what is open props?

thescientist13

OK, so I think I got this into a pretty good shape and I think just a couple small issues remain which I am tracking in the description, any contributions on the mobile menu bugs or any final styling alignments would be much appreciated! 🙇

Will try and circle back by the end of the week to get this merged and knock out with some basic test cases too. 👍

Auhseh · 2024-05-14T05:16:32Z

OK, so I think I got this into a pretty good shape and I think just a couple small issues remain which I am tracking in the description, any contributions on the mobile menu bugs or any final styling alignments would be much appreciated! 🙇

Will try and circle back by the end of the week to get this merged and knock out with some basic test cases too. 👍

Tweaked the design to fall back on the nearest open-props values. Also reflected it in the code.

Only issue in the nearest future might be the Landing page Hero text which is 9.25rem (And integral to the Visual style of the page), whereas the largest open-props value seems to be 3.5rem

Fixed bugs and mobile nav

Fixed Overlay

thescientist13

Looking good now, and I tested these changes and the behavior looks to be working as expected now! 💯

Just found a couple refactoring / SSR tweaks to make which I will push up in a few and then I think this should be good to merge.

thescientist13 · 2024-05-17T18:32:20Z

src/components/header/header.js

+    this.shadowRoot.adoptedStyleSheets = [sheet];
+
+    // Mobile menu toggle
+    const mobileMenu = this.shadowRoot.querySelector(".mobile-menu");


looks like the optional chaining references were removed, e.g.

const mobileMenu = this.shadowRoot?.querySelector(".mobile-menu");

thescientist13 · 2024-05-17T18:36:19Z

src/components/header/header.js

+      socialTray.style.display = isMobileMenuActive ? "none" : "flex";
+    }
+
+    mobileMenu.addEventListener("click", function () {


Will probably want to guard addEventListener here for SSR as well, and we can probably just pass the function reference directly

closeButton?.addEventListener("click", toggleMobileMenu);

thescientist13 · 2024-05-17T18:36:45Z

src/components/header/header.js

+      }
+    }
+
+    window.addEventListener("resize", handleResize);


We'll want to reference globalThis in place of window for better SSR / prerendering support

thescientist13 · 2024-05-17T18:59:04Z

src/components/header/header.js

+      toggleMobileMenu();
+    });
+
+    function handleResize() {


Since custom elements are classes, it might be useful for us to leverage method and members for these variables, which could help us clean up the connectedCallback function a bit.

thescientist13

Looking good! Just left a couple final comments but we can track those as their own issues if need be.

thescientist13 · 2024-05-17T19:56:42Z

src/components/header/header.css

+  box-sizing: border-box;
+  height: var(--size-10);
+  width: auto;
+  border: var(--border-size-1) dotted #989898ca;


@Auhseh
Just curious, I see a few of these 8 character hex values, is this a new extension in CSS? Linters and VSCode don't seem to complain but I'm probably just a little out of touch, hah.

Aside from that, it does look like there are a couple of these repeated values through this CSS, perhaps we could create some locally scoped values using :host selector at the top of this file?

:host { --border-color-gray: #989898ca }

Or they can be added to theme.css if we expect to use them in other places too.

Sorry for the lag here, I was unwell and I’m just now recuperating… I’m back now.

The extra two characters essentially just tweak the opacity..
The :host idea looks neat

thescientist13 · 2024-05-17T19:57:40Z

src/components/header/header.js

+
+          <nav class="nav-bar">
+            <ul class="nav-bar-menu">
+              <li class="nav-bar-menu-item">


Do we want to leverage any visited selector styles for our links? Not sure if that's best practice or not, but they do show as purple for me (naturally)

Not sure what best practices are here

cc: @aholtzman

thescientist13 requested changes May 12, 2024

View reviewed changes

thescientist13 added docs Greenwood specific content like docs and guides enhancement New feature or request labels May 12, 2024

thescientist13 linked an issue May 12, 2024 that may be closed by this pull request

Header Component #26

Closed

7 tasks

thescientist13 changed the title ~~Create header.js~~ feature/issue 26 header component May 12, 2024

thescientist13 reviewed May 12, 2024

View reviewed changes

Auhseh and others added 22 commits May 13, 2024 11:19

Create header.js

8b75ce2

Update header.js

807d95a

Update header.js

ba72b84

Update header.js

eb75476

Update theme.css

3c13dac

Update header.js

a367c50

Create blog-landing.html

6f7c3fb

Add files via upload

e391143

Update blog-landing.html

9d95d98

Update blog-landing.html

f4f03ab

Update main.css

a7bab37

Add files via upload

26ef22e

Create blog-post

35db879

Delete src/pages/blog/blog-post

f0dbe51

Delete src/pages/blog/blog-landing.html

e85dfc3

Delete src/assets/Product-update-tag.svg

259ee48

Delete src/assets/article-placeholder.svg

1fe8cc0

Delete src/assets/article-placeholder-thumbnail.svg

3acc514

Update header.js

912ce87

header storybook

063734b

remove unrelated styles

f4eb536

storybook and constructable stylesheets compat

66e3bd0

thescientist13 added 4 commits May 13, 2024 16:05

refactor vite configuration

6d0d013

add site and social links

15afc12

use HTML entity

929b34e

refactor hardcoded conditions

6c9504a

thescientist13 added the question Further information is requested label May 13, 2024

thescientist13 added 3 commits May 13, 2024 16:27

add storybook static dirs config

3428472

handle build conditions

63b5685

convert header icons to use local SVG files

e0e15f8

This was referenced May 13, 2024

upstream feature / fixes tracking #37

Closed

optimize header component for static content / Light DOM #48

Closed

support raw transforms for WTR

4c2a37e

thescientist13 requested a review from aholtzman May 13, 2024 22:26

thescientist13 reviewed May 13, 2024

View reviewed changes

thescientist13 marked this pull request as ready for review May 13, 2024 22:30

Update header.css

883a5b8

Auhseh added 5 commits May 14, 2024 08:35

Update header.js

a0c23c7

Fixed bugs and mobile nav

Update header.css

decc70e

Fixed Overlay

Update header.css

c9f0738

Update header.js

ac1b314

Update header.js

701cc71

thescientist13 reviewed May 17, 2024

View reviewed changes

thescientist13 mentioned this pull request May 17, 2024

create a shared layout for all pages #32

Closed

8 tasks

thescientist13 removed the question Further information is requested label May 17, 2024

thescientist13 added 3 commits May 17, 2024 15:08

misc refactoring

7018c9b

misc cleanup

69e8829

add test cases for the header

620f6c3

thescientist13 approved these changes May 17, 2024

View reviewed changes

thescientist13 merged commit 6802dcd into main May 23, 2024
5 checks passed

thescientist13 deleted the Auhseh-patch-1 branch May 23, 2024 00:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/issue 26 header component #44

feature/issue 26 header component #44

Auhseh commented May 5, 2024 •

edited by thescientist13

Loading

netlify bot commented May 5, 2024 •

edited

Loading

thescientist13 left a comment •

edited

Loading

thescientist13 May 12, 2024

Auhseh May 13, 2024

thescientist13 May 12, 2024

thescientist13 May 12, 2024

Auhseh May 13, 2024

thescientist13 May 12, 2024

thescientist13 May 12, 2024

thescientist13 May 12, 2024

Auhseh May 13, 2024

thescientist13 May 13, 2024

thescientist13 left a comment •

edited

Loading

Auhseh commented May 14, 2024

thescientist13 left a comment

thescientist13 May 17, 2024

thescientist13 May 17, 2024

thescientist13 May 17, 2024

thescientist13 May 17, 2024

thescientist13 left a comment

thescientist13 May 17, 2024

Auhseh May 30, 2024

thescientist13 May 17, 2024

		/-----------------------------------------------------------------/


		/-----BLOG STYLING--------------------------------------------------/

feature/issue 26 header component #44

feature/issue 26 header component #44

Conversation

Auhseh commented May 5, 2024 • edited by thescientist13 Loading

Related Issue

Summary of Changes

TODOs

Nice to have / separate issue

netlify bot commented May 5, 2024 • edited Loading

✅ Deploy Preview for super-tapioca-5987ce ready!

thescientist13 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thescientist13 left a comment • edited Loading

Choose a reason for hiding this comment

Auhseh commented May 14, 2024

thescientist13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thescientist13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Auhseh commented May 5, 2024 •

edited by thescientist13

Loading

netlify bot commented May 5, 2024 •

edited

Loading

thescientist13 left a comment •

edited

Loading

thescientist13 left a comment •

edited

Loading