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

For vendors: Header section #504

Merged

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Dec 4, 2023

closes #495

What this PR does

  • Adds new content section (consisting of an SVG with an Alt tag, a title, paragraphs and a button)
  • This change uses ALL the same HTML/CSS as what the home page top section has.
  • Does this PR change any CSS? No.

Copy link

netlify bot commented Dec 4, 2023

Deploy Preview for cal-itp-mobility-marketplace ready!

Name Link
🔨 Latest commit 7ee9a99
🔍 Latest deploy log https://app.netlify.com/sites/cal-itp-mobility-marketplace/deploys/656e3f0e795fd00009e53e4a
😎 Deploy Preview https://deploy-preview-504--cal-itp-mobility-marketplace.netlify.app/for-vendors
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@machikoyasuda machikoyasuda changed the base branch from main to feat/499-for-vendors-newsletter December 4, 2023 21:05
src/for-vendors.html Outdated Show resolved Hide resolved
@machikoyasuda
Copy link
Member Author

There are some spacing issues here (like between paragraphs, buttons, and container alignment details), but would like to leave it to this #490 issue for a full update.

Example: I COULD rewrite ALL paragraphs to have the same margin/padding-bottom, but I'd need confirmation from @segacy1 that this change is okay for ALL paragraphs across ALL the pages. Or I could write really hacky CSS that does this: The current style says that there should be .7rem under each paragraph, which totals 11.2px. Then I have to subtract: 24px-11.2px to come up with what remainder padding I should add in addition to the .7, to make this page look like what is on Figma. Messy. cc @thekaveman @angela-tran

@machikoyasuda machikoyasuda self-assigned this Dec 4, 2023
@machikoyasuda machikoyasuda added this to the Provider Map Refresh milestone Dec 4, 2023
@thekaveman
Copy link
Member

but would like to leave it to this #490 issue for a full update.

Agree! Let's worry about all that at once.

@machikoyasuda machikoyasuda changed the title For Vendors: Header section For vendors: Header section Dec 4, 2023
@machikoyasuda machikoyasuda linked an issue Dec 4, 2023 that may be closed by this pull request
@machikoyasuda
Copy link
Member Author

machikoyasuda commented Dec 4, 2023

@thekaveman @angela-tran The code is ready to review. Otherwise, just awaiting a better alt image tag line.

Sidenote, more on the comment above - re: #490 - I tried briefly re-writing that HTML as regular Bootstrap container/row/column code, but the mobile implementation is more complex than that because of the way the mobile switches to a Header/Illustration/Text ordering. The styles on this project in general is really tricky to refactor, in part b/c the styles were so narrowly written to fit exactly what the first design had.

@segacy1
Copy link

segacy1 commented Dec 5, 2023

@machikoyasuda the png is a bit blurry, could we try it as an SVG?
vendor-hero-lg

@segacy1
Copy link

segacy1 commented Dec 5, 2023

@machikoyasuda for alt tag, how's this: A saleswoman behind a stand for "Better Products," framed by refresh and checkmark icon illustrations.

102 characters

@machikoyasuda machikoyasuda marked this pull request as ready for review December 5, 2023 17:24
@machikoyasuda machikoyasuda requested a review from a team as a code owner December 5, 2023 17:24
@machikoyasuda
Copy link
Member Author

With the inclusion of the alt tag for the illustration, this PR is now ready for review @thekaveman @angela-tran

@angela-tran
Copy link
Member

@machikoyasuda What do you think about using the SVG that Segacy uploaded in #504 (comment)? The PNG is blurry for me too

@machikoyasuda
Copy link
Member Author

@angela-tran @segacy1

The SVG is much less blurry - way better than the PNG:
image

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

🏪

@machikoyasuda machikoyasuda merged commit 31e17d6 into feat/499-for-vendors-newsletter Dec 7, 2023
1 check passed
@machikoyasuda machikoyasuda deleted the feat/495-for-vendors-header branch December 7, 2023 01:03
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.

[vendor page] Add new header image and link to contact us
4 participants