Skip to content

feat(v2): themed logo in footer#3993

Merged
slorber merged 7 commits into
facebook:masterfrom
natac13:footer-themed-logo
Jan 6, 2021
Merged

feat(v2): themed logo in footer#3993
slorber merged 7 commits into
facebook:masterfrom
natac13:footer-themed-logo

Conversation

@natac13

@natac13 natac13 commented Jan 4, 2021

Copy link
Copy Markdown
Contributor

Motivation

Trying to get the Logo in Footer Themed

PR Ready, Needs srcDark photo removed before merge and deploy. This is left as instructed
Closes #3989

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

Related PRs

#3989 (comment)

@natac13 natac13 requested review from lex111 and slorber as code owners January 4, 2021 18:18
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Hi @natac13!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@netlify

netlify Bot commented Jan 4, 2021

Copy link
Copy Markdown

✔️ Deploy preview for docusaurus-2 ready!
Built without sensitive environment variables

🔨 Explore the source changes: fb0ad00

🔍 Inspect the deploy logs: https://app.netlify.com/sites/docusaurus-2/deploys/5ff5d0bfc7223f0007e5ea77

😎 Browse the preview: https://deploy-preview-3993--docusaurus-2.netlify.app

@github-actions

github-actions Bot commented Jan 4, 2021

Copy link
Copy Markdown

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 87
🟢 Accessibility 99
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-3993--docusaurus-2.netlify.app/classic/

@github-actions

github-actions Bot commented Jan 4, 2021

Copy link
Copy Markdown

Size Change: +5 B (0%)

Total Size: 156 kB

ℹ️ View Unchanged
Filename Size Change
website/build/blog/2017/12/14/introducing-docusaurus/index.html 20.7 kB -1 B (0%)
website/build/docs/introduction/index.html 180 B 0 B
website/build/index.html 5.82 kB -1 B (0%)
website/build/main.********.js 112 kB +7 B (0%)
website/build/styles.********.css 17.6 kB 0 B

compressed-size-action

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jan 4, 2021
@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@slorber

slorber commented Jan 4, 2021

Copy link
Copy Markdown
Collaborator

Hi @natac13

Can you temporarily add a fake darkSrc to our own website?

This will make the review process easier for me (and help understanding the problems you mention in your comment), I'll remove this before merging.

For now the code looks nice

Comment thread packages/docusaurus-theme-classic/src/theme/Footer/styles.module.css Outdated
@natac13

natac13 commented Jan 4, 2021

Copy link
Copy Markdown
Contributor Author

Hi @natac13

Can you temporarily add a fake darkSrc to our own website?

This will make the review process easier for me (and help understanding the problems you mention in your comment), I'll remove this before merging.

For now the code looks nice

Yep I can do this.

@natac13

natac13 commented Jan 4, 2021

Copy link
Copy Markdown
Contributor Author

Ok I go the image added to the website temporarily. It is working! Except for the centering. Sorry for the typos and multiple commits. Do you want them squashed? Or I can close this pr and start a fresh one with only one commit. Thanks for the help.

Update:

I believe if we can add the follow css to this div then the image will get centered!

    display: flex;
    flex-flow: column;
    align-items: center;

centered-footer-logo-docusaurus-v2

NOTE
I left the srcDark temp photo as instructed. Just wanted to mention this so you can remove it before ending up in the prod site.

opacity: 1;
}

.footerLogoImg {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead this, I would suggest within this PR, change block to inline in the stylesheet of ThemedImage component here https://github.com/facebook/docusaurus/blob/master/packages/docusaurus-theme-classic/src/theme/ThemedImage/styles.module.css

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

agree

would even use "initial" (but it's somehow the same)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Would this need to be changed in both classic theme and bootstrap theme?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the bootstrap theme is not too important right now, do not care about it ;)
I'll clean it up later when I'll be working on the theme gallery

@natac13 natac13 changed the title feat: attempt 1 at themed logo in footer feat: themed logo in footer Jan 4, 2021
@natac13 natac13 changed the title feat: themed logo in footer feat(v2): themed logo in footer Jan 4, 2021
@slorber

slorber commented Jan 5, 2021

Copy link
Copy Markdown
Collaborator

Not sure why but comparing before/after it seems the layout got a bit affected.

Also, the hover animation does not work anymore

@natac13

natac13 commented Jan 6, 2021

Copy link
Copy Markdown
Contributor Author

Changed the Themed Image styling as suggested. Works great!

Not sure why but comparing before/after it seems the layout got a bit affected.

Also, the hover animation does not work anymore

Not sure why the hover animation is not working for you. Here is a video of me testing it.

docusaurus-footer-logo.mp4

Also I am not sure what layout bit got affected.

@slorber

slorber commented Jan 6, 2021

Copy link
Copy Markdown
Collaborator

The recent changes seems to fix the layout and anim issues I had, so it looks good to merge 👍 thanks

@natac13

natac13 commented Jan 6, 2021

Copy link
Copy Markdown
Contributor Author

The recent changes seems to fix the layout and anim issues I had, so it looks good to merge thanks

Just a reminder about removing the temp srcDark image. I like my photo but I doubt you want it on the prod site lol!

Thanks for the help with this PR

Comment thread packages/docusaurus-theme-classic/src/theme/ThemedImage/styles.module.css Outdated
@slorber

slorber commented Jan 6, 2021

Copy link
Copy Markdown
Collaborator

Thanks, you can remove this picture now.

Was wondering why inherit instead of inline/initial.

I'd prefer initial, as it means we revert to "default", but maybe you have a good reason to use inherit instead?

This was my mistake.

Removed temp `srcDark` footer logo
@natac13

natac13 commented Jan 6, 2021

Copy link
Copy Markdown
Contributor Author

Sorry about the styling mistake. I got it change to initial and made sure things still looked good with layout and hover.

I have removed the temp srcDark image.

@slorber

slorber commented Jan 6, 2021

Copy link
Copy Markdown
Collaborator

thanks 👍

@slorber slorber merged commit 5dfa665 into facebook:master Jan 6, 2021
@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Jan 11, 2021
@lex111 lex111 added this to the v2.0.0-alpha.71 milestone Mar 1, 2021
This was referenced Mar 10, 2021
mrizwanashiq pushed a commit to mrizwanashiq/docusaurus that referenced this pull request Jun 25, 2026
* feat: attempt 1 at themedlogo in footer

* Add srcDark to website for temp debug

* typo fix

* Got it now! except for centering

* style: centered new themed image logo

* style: change ThemedImage styleing to center footer logo

* style: Change ThemeImage Style to initial

This was my mistake.

Removed temp `srcDark` footer logo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Footer logo doesn't allow srcDark

4 participants