style(frontend): changes footer copyright text #3588
Conversation
|
I am not sure I really like the mobile version: was it required like this? maybe more space between the icon and the sentence? or even putting "by Dfinity Foundation" in another row together with the year |
|
@AntonioVentilii Sry, you are right. The text needs to be centered. I just fixed it. |
|
@BonomoAlessandro may you please update the screenshots in the description? |
improves footer design
…opyright-text' into style(frontend)/changes-footer-copyright-text
| @@ -53,35 +53,44 @@ | |||
| </div> | |||
|
|
|||
| <div | |||
| class="item pointer-events-auto flex flex-row items-center justify-end gap-2 text-sm lg:max-w-48 xl:max-w-none" | |||
| class="item pointer-events-auto pl-6 pr-6 text-sm md:pl-0 md:pr-0 lg:max-w-48" | |||
There was a problem hiding this comment.
px-6 md:px-6?
There was a problem hiding this comment.
yes, in the Figma we do have a padding of 24px on the left and right that missed till now. But yeah, i can replace pl-6 and pr-6 with px-6.
| class:md:transition-all={$authSignedIn} | ||
| class:md:duration-200={$authSignedIn} | ||
| class:md:ease-in-out={$authSignedIn} | ||
| class:md:invisible={$authSignedIn} | ||
| class:1.5md:visible={$authSignedIn} | ||
| class:md:translate-x-full={$authSignedIn} | ||
| class:1.5md:translate-x-0={$authSignedIn} | ||
| class:xl:max-w-80={$authSignedIn} |
There was a problem hiding this comment.
Tbh, it's really hard to follow what's going on here. E.g. why lg:max-w-48 we set as a string above and xl:max-w-80 here? You can also move this value to the string definition and just keep class:xl:max-w-none={$authNotSignedIn}.
Also, could you please sort these values? Maybe by CSS rule, ideally with a line break between them. Just for the sake of readability.
There was a problem hiding this comment.
I know it is hard to understand what exactly is going on...
No, i can't move xl:max-w-80 to the string definition, because this would break the behaviour of the footer on the login page.
It would look like this:
but it should look like this:
There was a problem hiding this comment.
Just as an idea: maybe it would be easier to maintain this code if we do something like ${$authSignedIn ? 'sm:max-w-.. md:max-w:.. lg:max-w-... other-properties' : 'sm:max-w-.. md:max-w:.. lg:max-w-... other-properties'}? With this format, there will be less code, and most importantly a clear difference between 2 states.
There was a problem hiding this comment.
Personally i think it is easier to understand like it is, instead of ${$authSignedIn ? 'sm:max-w-.. md:max-w:.. lg:max-w-... other-properties' : 'sm:max-w-.. md:max-w:.. lg:max-w-... other-properties'}
I think the best solution would be to order it like you mentioned in the beginning.
There was a problem hiding this comment.
Alright, let's keep them sorted.
| class:md:transition-all={$authSignedIn} | ||
| class:md:duration-200={$authSignedIn} | ||
| class:md:ease-in-out={$authSignedIn} | ||
| class:md:invisible={$authSignedIn} | ||
| class:1.5md:visible={$authSignedIn} | ||
| class:md:translate-x-full={$authSignedIn} | ||
| class:1.5md:translate-x-0={$authSignedIn} | ||
| class:xl:max-w-80={$authSignedIn} | ||
| class:1.5xl:max-w-none={$authSignedIn} |
There was a problem hiding this comment.
Why this one is needed? I don't see a contrary value for the 1.5xl breakpoint that you need to overwrite.
There was a problem hiding this comment.
With this command i say that if the screen is bigger than the defined size 1.5xl than there is no limitation in the width anymore.
If you are logged in it will override the max-w-80 otherwise it will override max-w-48.
| class:md:h-md:hidden={$authNotSignedIn} | ||
| class:1.5md:h-md:flex={$authNotSignedIn} | ||
| <div class="flex flex-col items-center pt-2 sm:flex-row sm:items-start sm:gap-2"> | ||
| <span class="-mt-[0.35rem]"><IconDfinity size="30" /></span> |
There was a problem hiding this comment.
What's the reason for negative margin? To align in some custom way the icon with the text? Maybe some line-height for the text could do the job?
There was a problem hiding this comment.
Yes exactly, this is used to aligne the icon with the text. The line-height for the text did not work as needed so i had to use this.
| @@ -53,35 +53,44 @@ | |||
| </div> | |||
|
|
|||
| <div | |||
| class="item pointer-events-auto flex flex-row items-center justify-end gap-2 text-sm lg:max-w-48 xl:max-w-none" | |||
| class="item pointer-events-auto px-6 text-sm md:pl-0 md:pr-0 lg:max-w-48" | |||
There was a problem hiding this comment.
Padding can be shortened for md too: md:px-0.
There was a problem hiding this comment.
@DenysKarmazynDFINITY
You are right. I just fixed it.
BonomoAlessandro
deleted the
style(frontend)/changes-footer-copyright-text
branch
Motivation
Instead of the text
© 2024 Developed with ❤️ at DFINITYthe textIncubated with ❤️ by DFINITY Foundation © 2024should be displayed as copyright text in the footer.Changes
Tests
before:
https://github.com/user-attachments/assets/4c48d02f-6de4-4f7e-8127-80dde42491e4
after:
https://github.com/user-attachments/assets/0c48e243-6505-4795-b875-9b42c86c98d2