Skip to content

refactor(v2): introduce Logo component, remove useLogo hook#3745

Merged
slorber merged 3 commits into
facebook:masterfrom
Simek:refactor-theme-classic-logo
Nov 16, 2020
Merged

refactor(v2): introduce Logo component, remove useLogo hook#3745
slorber merged 3 commits into
facebook:masterfrom
Simek:refactor-theme-classic-logo

Conversation

@Simek

@Simek Simek commented Nov 13, 2020

Copy link
Copy Markdown
Contributor

Motivation

Since the ThemedImage component has been added, it would be nice to utilize it in the theme files. First thing that came to my mind was the site logo component. After studying a code for a bit, it came apparent that very similar code is used in the all places, so it can be quite easily extracted to the separate file.

When I have finished the extraction, the next thing that picked up my attention was useLogo hook, which after the refactor seems redundant too (code can be moved to the newly created Logo component).

This hook has not been documented yet on the website - https://v2.docusaurus.io/docs/theme-classic#hooks - (in opposition to the theme-bootstrap hook - https://v2.docusaurus.io/docs/theme-bootstrap/#uselogo), so I think that the removal shouldn't hurt any users. This change also makes code structure a bit simpler.

In addition to the changes described above this refactor fixes a HTML check warning when user do not specified the logo.alt field in the navbar config (fallback to title, then just to "Logo" string).

Unfortunately the sources had to be extracted to the variable due to Rule of Hooks conditional call warnings.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Refactor has been tested on the Docusuaurs V2 website locally.

Related PRs

@Simek Simek requested review from lex111 and slorber as code owners November 13, 2020 19:35
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 13, 2020
@netlify

netlify Bot commented Nov 13, 2020

Copy link
Copy Markdown

Deploy preview for docusaurus-2 ready!

Built without sensitive environment variables with commit f49c56c

https://deploy-preview-3745--docusaurus-2.netlify.app

@netlify

netlify Bot commented Nov 13, 2020

Copy link
Copy Markdown

Deploy preview for docusaurus-2 ready!

Built without sensitive environment variables with commit 41afde2

https://deploy-preview-3745--docusaurus-2.netlify.app

@lex111 lex111 left a comment

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.

Good idea! 👍

@lex111 lex111 added the pr: polish This PR adds a very minor behavior improvement that users will enjoy. label Nov 13, 2020
@lex111 lex111 changed the title fix(v2): refactor theme-classic logo, remove useLogo hook refactor(v2): introduce Logo component, remove useLogo hook Nov 13, 2020
@slorber

slorber commented Nov 16, 2020

Copy link
Copy Markdown
Collaborator

LGTM thanks, happy to remove that useLogo hook

@slorber slorber merged commit 3ad4539 into facebook:master Nov 16, 2020
@lex111 lex111 added this to the v2.0.0-alpha.67 milestone Nov 17, 2020
mrizwanashiq pushed a commit to mrizwanashiq/docusaurus that referenced this pull request Jun 25, 2026
…#3745)

* fix(v2): refactor theme-classic logo, remove useLogo hook

* Logo component small code tweaks

* Logo component returned type fix
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: polish This PR adds a very minor behavior improvement that users will enjoy.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants