search

gevuong / minimal-i18n-with-app-router

Building the marketing site so it's deployable to GPages @virufy. It's statically exported, supports build time image optimization and internationalized routing with App Router.
https://gevuong.github.io/minimal-i18n-with-app-router/
MIT License
0 stars 5 forks source link

55 style 3 footer modals to match design system #58

Closed a-wong-8 closed 1 month ago

a-wong-8 commented 2 months ago
  1. styled footer modals to match figma
  2. reused AccordianItem component from FAQ for footer modals
netlify[bot] commented 2 months ago

Deploy Preview for adorable-fairy-ecd9fc ready!

Name Link
Latest commit 83d2e591f0709934958ad5efd6af0825df2fc026
Latest deploy log https://app.netlify.com/sites/adorable-fairy-ecd9fc/deploys/66d87ec78ce86d0008b81de3
Deploy Preview https://deploy-preview-58--adorable-fairy-ecd9fc.netlify.app
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.

a-wong-8 commented 2 months ago

Due to limited time, I'm not able to review the entire PR but this is what I gathered:

  1. I don't recommend adding HTML w/ Tailwind in metadata because we want separation of concern between metadata, HTML w/ Tailwind. If someone needed to update the styling, HTML, or debug, we want them to view and edit the React component, not the metadata. If there isn't a better solution, I would revert it to what it was before.
  2. Are there alternatives to using dangerouslySetInnerHTML? Based on research, it's typically avoided unless it's necessary. In the case of simply rendering static content in a modal, I would prefer using an alternative method of rendering content.
  3. Accordion is misspelled.

@gevuong ok items corrected

a-wong-8 commented 2 months ago
  1. I changed the max-h- to max-h-full
  2. added left padding
  3. fixed max-h for 6th question
  4. i'm not sure why the transition does not work most of the time - I think it has to do with the max-h. the arrow looks ok on my end (chrome). any suggestions for the transition?
  5. this might take too much time