Local footer styling to match Stanford Basic subtheme #5

jenbreese · 2024-03-12T22:19:03Z

NOT READY FOR REVIEW

(Edit the above to reflect status)

Summary

Added the web login button. It needs the correct saml login link. I added it to the bottom of the local footer like ChEM-H did.
Added font sizing.
Added spacing.
Added the subscription form.
I grouped and commented what the different variables are because the names are confusing.
Questions for @pookmish
-- How can we include the lockup that we use in the header? Is there a way to make the footer one use the main one in the story? I read a little about storybook compositions. Maybe that is what I'm looking for?

Review By (Date)

When does this need to be reviewed by?

Criticality

How critical is this PR on a 1-10 scale? Also see Severity Assessment.
E.g., it affects one site, or every site and product?

Urgency

How urgent is this? (Normal, High)

Review Tasks

Setup tasks and/or behavior to test

Check out this branch
Rebuild Cache and import config drush cr ; drush ci
Navigate to...
Verify...

Site Configuration Sync

Is there a config:export in this PR that changes the config sync directory?

Front End Validation

Design is approved by @ user?
HTML validation: Is the markup using the appropriate semantic tags and passes validation? Or, QA request ticket created?
Cross-browser testing: Has been performed? Or, QA request ticket created?
Automated accessibility: Scans performed? Or, QA request ticket created?
Manual accessibility: Manually tested? Or, QA request ticket created?

Backend / Functional Validation

Code

Are the naming conventions following our standards?
Does the code have sufficient inline comments?
Is there anything in this code that would be hidden or hard to discover through the UI?
Are there any code smells?
Are tests provided? eg (unit, behat, or codeception)

Code security

Are all forms properly sanitized?
Any obvious security flaws or new areas for attack?

General

Is there anything included in this PR that is not related to the problem it is trying to solve?
Is the approach to the problem appropriate?

Affected Projects or Products

Does this PR impact any particular projects, products, or modules?

Associated Issues and/or People

JIRA ticket(s)
Other PRs
Any other contextual information that might be helpful (e.g., description of a bug that this PR fixes, new functionality that it adds, etc.)
Anyone who should be notified? (@mention them here)

Resources

AMP Tool
Accessibility Manual Test Script
HTML Validator
Browserstack and link to Browserstack Credentials

jenbreese · 2024-04-03T02:49:57Z

@pookmish Can you take a look at this PR and offer suggestions for improvement?

pookmish · 2024-04-03T02:56:14Z

src/components/elements/web-login.tsx

+import {LockClosedIcon} from "@heroicons/react/24/outline";
+import Link from "next/link";
+
+const WebLogin = () => {


I'm good with most of this PR, except this component. Since it's a decoupled site, we'll basically be hiding the web login and hiding the drupal side. At this time I'd like to remove this.

Ok. That makes sense. I removed it.

pookmish · 2024-04-03T02:58:22Z

src/components/config-pages/local-footer.tsx

+          <div>
+            <div className="su-signup-form">
+              <Wysiwyg html={suLocalFootFIntro?.processed}/>
+              <Subscribe/>
+            </div>

-          <Wysiwyg html={suLocalFootTrCo?.processed}/>
-
-        </div>
+            <Wysiwyg html={suLocalFootTrCo?.processed} className="text-19"/>
+          </div>


This section could probably have more simple markup:

<div> <Wysiwyg html={suLocalFootFIntro?.processed}/> <Subscribe/> <Wysiwyg html={suLocalFootTrCo?.processed} className="text-19"/> </div>

And even more so, does the wrapping div need to be there?

The wrapping is not needed and had some wrong thinking behind it.

pookmish · 2024-04-03T02:59:30Z

src/components/elements/address.tsx

@@ -38,7 +38,7 @@ const Address = ({
  }

  return (
-    <address {...props}>
+    <address {...props} className="text-16 leading-9 rs-mb-3">


Remember that the Address element is used across the whole site. If you want to apply these classes in the footer, you should apply them where the component is used. That's what the {...props} allows us to do

Ok. I moved it to the footer instead. I'll look again in the am. I might have missed the point.

pookmish · 2024-04-03T03:01:24Z

src/components/elements/subscribe.tsx

+    <form>
+    <input className="h-[3.5rem] w-[25rem] block type-0" type="email" id="" aria-label="signup email" name="signup email" placeholder="email address" />
+    <button className="button m-t-3 my-2 h-[35px]" type="submit" id="signup-submit">
+      Sign-up
+    </button>


This form wouldn't submit anywhere the way it is. At this time, we should just remove it. I don't believe people use the local footer signup form anywhere anyways.

I don't think many sites do but ChEM-H does. They aren't going to use this at the moment. I removed it. It'd be interesting to know how many actually use it.

pookmish · 2024-04-03T15:42:45Z

.storybook/stories/config-pages/LocalFooter.stories.tsx

@@ -3,7 +3,9 @@ import type {Meta, StoryObj} from '@storybook/react';
 import LocalFooter from "@components/config-pages/local-footer";
 import {ComponentProps} from "react";

-type ComponentStoryProps = ComponentProps<typeof LocalFooter> & {}
+type ComponentStoryProps = ComponentProps<typeof LocalFooter> & {
+  numberoflinks: number


For ease of reading and consistent code styles, it's good to use camel case characters. numberOfLinks

I changed the name of numberOfLinks and noticed I had the same style problem in the gallery page so I fixed that too.

jenbreese · 2024-04-03T20:43:48Z

.storybook/stories/config-pages/LocalFooter.stories.tsx

+    for (let i=0; i< numberOfLinks; i++) {
+      linkList.push("<a href='http://localhost'>Primary Link</a>")
+    }
+    return <LocalFooter {...args} />


@pookmish Can you point out where this is wrong? I thought I'd be able to pass the linkList into the return like <LocalFooter {linkList, ...args} /> but that was an error. I want the control to just add more links so the client can see what happens in the display when they add lots of links.

you're creating an array, but you're not doing anything with the array. Also links are typically structured objects, so you'd need to do something like linkList.push({url: "http:/foobar", title: "foo"})
But then once you build the array, you need to set it to the args. args.configPage.[your_field_name] = linkList

Do you mean like this?

args: { configPage: { suLocalFootAction: linkList, },

pookmish

This still requires fixes to the storybook story

pookmish requested changes Apr 3, 2024

View reviewed changes

pookmish reviewed Apr 3, 2024

View reviewed changes

jenbreese commented Apr 3, 2024

View reviewed changes

pookmish approved these changes Apr 5, 2024

View reviewed changes

jenbreese added 13 commits April 5, 2024 11:22

Local footer styling to match Stanford Basic subtheme

6f0c2d6

stashing work

a709d1d

local footer updates

e804c8c

fixup to icon

167fbf0

fixup for the footer. worked on subscribe button

e6aac98

link fixup

250ef60

adding social icons

ce917c9

updating twitter icon

a5dce51

lint fixup

a87b874

updates based on feedback

65a8c02

feedback updates

7b81e1b

partially fixed

af064e6

fixup

d61b08b

pookmish force-pushed the local-footer-styling branch from de0308a to d61b08b Compare April 5, 2024 18:24

pookmish requested changes Apr 5, 2024

View reviewed changes

pookmish force-pushed the 1.x branch 2 times, most recently from 6524635 to a9f4c29 Compare July 10, 2024 19:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Local footer styling to match Stanford Basic subtheme #5

Local footer styling to match Stanford Basic subtheme #5

jenbreese commented Mar 12, 2024 •

edited

Loading

jenbreese commented Apr 3, 2024

pookmish Apr 3, 2024

jenbreese Apr 3, 2024

pookmish Apr 3, 2024

jenbreese Apr 3, 2024

pookmish Apr 3, 2024

jenbreese Apr 3, 2024

pookmish Apr 3, 2024

jenbreese Apr 3, 2024

pookmish Apr 3, 2024

jenbreese Apr 3, 2024

jenbreese Apr 3, 2024

pookmish Apr 3, 2024

jenbreese Apr 3, 2024

pookmish left a comment

Local footer styling to match Stanford Basic subtheme #5

Are you sure you want to change the base?

Local footer styling to match Stanford Basic subtheme #5

Conversation

jenbreese commented Mar 12, 2024 • edited Loading

NOT READY FOR REVIEW

Summary

Review By (Date)

Criticality

Urgency

Review Tasks

Setup tasks and/or behavior to test

Site Configuration Sync

Front End Validation

Backend / Functional Validation

Code

Code security

General

Affected Projects or Products

Associated Issues and/or People

Resources

jenbreese commented Apr 3, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pookmish left a comment

Choose a reason for hiding this comment

jenbreese commented Mar 12, 2024 •

edited

Loading