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

NavbarLogo doesn't have width and height attribute #5724

Closed
5 tasks done
sohamsshah opened this issue Oct 16, 2021 · 4 comments · Fixed by #5770
Closed
5 tasks done

NavbarLogo doesn't have width and height attribute #5724

sohamsshah opened this issue Oct 16, 2021 · 4 comments · Fixed by #5770
Labels
bug An error in the Docusaurus core causing instability or issues with its execution

Comments

@sohamsshah
Copy link
Contributor

🐛 Bug Report

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io
  • I have read the console error message carefully (if applicable)

Description

The img tag in the NavbarLogo attribute in docusaurus.config.js file to add Logo to Navbar doesn't have any option/prop to set the height and width of the logo. Because of this, the Lighthouse Report fails and suggests to add width and height to the logo image.

Have you read the Contributing Guidelines on issues?

Yes, I have Read it.

Steps to reproduce

  1. Run Lighthouse test on any docusaurus powered website
  2. See the diagnostics message that suggests to add width and height attribute
  3. Also in inspect element, width and height attributes are missing.

Expected behavior

There should be props for this in order to add height and width to image along with defaults.

Screen Shot

image

Your environment

  • Docusaurus version used: latest
  • Environment name and version (e.g. Chrome 78.0.3904.108, Node.js 10.17.0): Chrome 87
  • Operating system and version (e.g. Ubuntu 20.04.2 LTS): Windows 10

I am new to the community and willing to contribute towards this issue. Please let me know more insights upon this. Would like to take this up!

@sohamsshah sohamsshah added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Oct 16, 2021
@Josh-Cena
Copy link
Collaborator

As a further request: can we make all images within a Markdown file have the height (and width, maybe) attribute as well? Is it possible to fetch this data when we convert them to require calls?

This ensures that there's no layout shift as the images load.

@slorber
Copy link
Collaborator

slorber commented Oct 20, 2021

Yes, agree that we should definitively do that!

@sohamsshah
Copy link
Contributor Author

Yes, agree that we should definitively do that!

Hi, Can you provide some implementation insights for me to get started?

@slorber slorber removed the status: needs triage This issue has not been triaged by maintainers label Oct 21, 2021
@slorber
Copy link
Collaborator

slorber commented Oct 21, 2021

Unfortunately not much, it's not something we have ever done yet in Docusaurus.

Maybe we can look into converting this:

module.exports = {
  themeConfig: {
    navbar: {
      title: 'Site Title',
      logo: {
        src: 'img/logo.png',
        srcDark: 'img/logo_dark.png',
      },
    },
  },
};

Into this?

module.exports = {
  themeConfig: {
    navbar: {
      title: 'Site Title',
      logo: {
        src: {src: 'img/logo.png', width: 50, height: 50},
        srcDark: {src: 'img/logo.png', width: 50, height: 50},
      },
    },
  },
};

Not sure where this conversion should lead exactly, how expensive it is, and also we must consider that some people provide an absolute URL, or a SVG...

Temporarily, we could allow providing width/height explicitly?

module.exports = {
  themeConfig: {
    navbar: {
      title: 'Site Title',
      logo: {
        src: 'img/logo.png',
        width: 50,
        height: 50,
        className: "my-custom-logo"
      },
    },
  },
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants