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

Reskin - home page footer #1636

Merged
merged 4 commits into from
May 7, 2024

Conversation

mudrila
Copy link
Contributor

@mudrila mudrila commented May 3, 2024

@mudrila mudrila added enhancement New feature or request re-skin Temporary Label for Reskin Work labels May 3, 2024
@mudrila mudrila self-assigned this May 3, 2024
@mudrila mudrila changed the base branch from develop to reskin/refactor-root-reskin May 3, 2024 20:25
Copy link

netlify bot commented May 3, 2024

Deploy Preview for fractal-dev ready!

Name Link
🔨 Latest commit 198431f
🔍 Latest deploy log https://app.netlify.com/sites/fractal-dev/deploys/663a2977c9173800076fc68f
😎 Deploy Preview https://deploy-preview-1636.app.dev.fractalframework.xyz
📱 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.

Copy link

@nicolaus-sherrill nicolaus-sherrill left a comment

Choose a reason for hiding this comment

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

oolala, new footer looks great!

I'm thinking it would be helpful to include copy to label the build #?
ex. Build No. cf19623

I'm not actually quite sure what that copy is supposed to be, but a label there would help.

Also the URL it is linked to goes to a 404 in Github.

@mudrila
Copy link
Contributor Author

mudrila commented May 3, 2024

oolala, new footer looks great!

I'm thinking it would be helpful to include copy to label the build #? ex. Build No. cf19623

I'm not actually quite sure what that copy is supposed to be, but a label there would help.

Also the URL it is linked to goes to a 404 in Github.

@nicolaus-sherrill
This 7 symbols is in fact first 7 symbols of commit hash, so it's not number of "release" or build but reference to the exact place in "code history" that being deployed to certain environment. Atm we're not marking our builds with releases - we're not scoping releases - anything that gets into main branch will be built into production environment.
Given that - I think it will make sense to label it with "Build Hash". Or maybe not even label that but add a tooltip where we can say "Latest/deployed/built commit hash: 1985843dcc0513af4ae17f4ac8f2f14315366a2b" and linking to the exact commit on GitHub.

Broken link is kinda expected atm - it links to decent-interface repo which in fact does not exist yet. It will work though when we'll be renaming repositories/packages (which will happen before releasing to prod ofc)

Lmk your thoughts

@mudrila mudrila requested a review from nicolaus-sherrill May 3, 2024 21:46
Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

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

Love it, ship it

Copy link
Contributor

@Da-Colon Da-Colon left a comment

Choose a reason for hiding this comment

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

🔥 Can't wait to open this app up soon to see how this work yall have done!

@@ -618,3 +618,44 @@ export function DocsIllustration() {
</svg>
);
}

export function DecentFooterLogo() {
Copy link
Contributor

Choose a reason for hiding this comment

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

(minor). We could use createIcon from @chakra-ui/react. Which would allow this component to act like any other Chakra-Icon with all the bells and whistles as needed. Probably not needed in this instance but thought I'd through it out there.

Here is an example from the design repo:
https://github.com/decentdao/decent-ui/blob/develop/design/atoms/components/icons/src/AddPlus.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah in this specific case this logo will be thrown away pretty soon 😄

Copy link

@xraystyle1980 xraystyle1980 left a comment

Choose a reason for hiding this comment

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

Now that I see it in production, I think we should expand the footer on larger desktop views outside of it's current container and allow it fill the viewport entirely. This would mimic the behavior of the top nav.

image

Updated in Figma
CleanShot - Figma - 2024-05-06@2x

@mudrila
Copy link
Contributor Author

mudrila commented May 7, 2024

Now that I see it in production, I think we should expand the footer on larger desktop views outside of it's current container and allow it fill the viewport entirely. This would mimic the behavior of the top nav.

image

Updated in Figma CleanShot - Figma - 2024-05-06@2x

Done, sir

@mudrila mudrila requested a review from xraystyle1980 May 7, 2024 13:15
@mudrila mudrila merged commit b9c3731 into reskin/refactor-root-reskin May 7, 2024
7 checks passed
@mudrila mudrila deleted the reskin/home-page-footer branch May 7, 2024 13:24
adamgall pushed a commit that referenced this pull request May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request re-skin Temporary Label for Reskin Work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reskin home page footer
6 participants