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

Update 404 page #122

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Update 404 page #122

merged 1 commit into from
Oct 21, 2024

Conversation

secbajor
Copy link
Contributor

Screenshot 2024-10-14 at 8 20 32 AM

[#57]

Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for fanciful-lamington-ea5ee2 ready!

Name Link
🔨 Latest commit dfb0c4d
🔍 Latest deploy log https://app.netlify.com/sites/fanciful-lamington-ea5ee2/deploys/6716cea03655a30008be26e0
😎 Deploy Preview https://deploy-preview-122--fanciful-lamington-ea5ee2.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.

</div>
<a
href="/"
className="px-5 py-2 my-6 text-base rounded-xl inline-block font-bold transition-all bg-breadviolet-shaded dark:bg-breadpink-shaded text-breadgray-ultra-white dark:text-breadgray-og-dark disabled:bg-opacity-60"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I combined all the relevant button classes to achieve this with an<a> tag since its just a link to home page -- but not sure if there's a better way to have done this

Copy link
Member

Choose a reason for hiding this comment

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

Looks good although there is a reusable <Button> component in core/components and I think the styles are the same.

I'd probably suggest sharing the styles and exporting a <ButtonLink> component from the same file so it's in the same place if styles change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, sounds right!

I made a new ButtonLink component which just imports things already defined in Button, but renders an a tag.

I'm blocked from merging until you post an approval on a review - can you take a quick look at my latest changes and if approve if all looks good?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@subject026 whoops, remembered the process is just to merge to dev. will do that now!

@secbajor
Copy link
Contributor Author

This was reviewed on today's design call and given approvals for moving forward.

@subject026 can you review this PR and let me know and advice/feedback you have on the code?

@secbajor secbajor changed the base branch from development to main October 21, 2024 22:08
@secbajor secbajor changed the base branch from main to development October 21, 2024 23:17
@secbajor secbajor merged commit fa3d1ae into development Oct 21, 2024
6 checks passed
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