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

Add ucsf partner logo #1307

Merged
merged 9 commits into from
Oct 23, 2023
Merged

Add ucsf partner logo #1307

merged 9 commits into from
Oct 23, 2023

Conversation

schroerbrian
Copy link
Contributor

Made updates to the home page partners. These include markup/CSS changes for the list of logos. Adding that extra logo causes them to wrap to the next line, which made them look awkward with our current setup.

I also added the logo to the SFSG about page as well as the LinkSF page. I'm not exactly sure if we need to keep the LinkSF page updated or even include the UCSF logo there, but it contains our other logos, so I added it for the time being. I'm not sure we get much traffic on LinkSF as it is, anyhow.

I think the UI that we currently have is not perfect, and it can look awkward on smaller screens, but maybe we should wait until our exec discussion before perfecting the logo UI.
Screen Shot 2023-10-20 at 3 59 04 PM

Screen Shot 2023-10-20 at 3 52 58 PM

Copy link
Member

@richardxia richardxia left a comment

Choose a reason for hiding this comment

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

LGTM, I left a few detailed CSS-related comments, but I don't think they are blockers for this to merge.

@@ -154,17 +154,17 @@ export const LinkSf = () => (
</li>
<li>
<a href="http://evictiondefense.org/" rel="noopener">
<img src={EDCLogo} alt="WeWork" />
<img src={EDCLogo} alt="Eviction Defense Collaborative" />
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for fixing the copy paste issues with the alt text everywhere!

app/pages/AboutPage/AboutPageMarkup/LinkSf.tsx Outdated Show resolved Hide resolved
app/pages/AboutPage/AboutPageMarkup/SfServiceGuide.tsx Outdated Show resolved Hide resolved
app/pages/AboutPage/AboutPage.module.scss Outdated Show resolved Hide resolved
@schroerbrian
Copy link
Contributor Author

LGTM, I left a few detailed CSS-related comments, but I don't think they are blockers for this to merge.

Thanks for reviewing! I've added your suggested changes, and everything looks better. Good catch noticing the effects of removing the header element from the logo li elements. See updated screenshots below:

Screen Shot 2023-10-23 at 12 11 34 PM

Screen Shot 2023-10-23 at 12 13 41 PM

@schroerbrian schroerbrian merged commit 0d34be3 into master Oct 23, 2023
4 checks passed
@schroerbrian schroerbrian deleted the add-ucsf-partner-logo branch October 23, 2023 19:23
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