backstage / backstage

Backstage is an open framework for building developer portals
https://backstage.io/
Apache License 2.0
28.59k stars 6.08k forks source link

🐛 Bug Report: Redundant Scrollbars in TechDocs on macOS Chrome and Firefox #22745

Closed sapslaj closed 5 months ago

sapslaj commented 9 months ago

📜 Description

I'm not sure if this affects other operating systems, but in Chrome and Firefox on macOS there are redundant scrollbars for each sidebar on a TechDocs page.

👍 Expected behavior

Safari seems to render correctly:

image

Note the lack of a scrollbar on the left sidebar and only a single scrollbar on the right side for the Table of contents.

👎 Actual Behavior with Screenshots

In Firefox:

image

In Chrome (with light mode):

image

To clarify, the issue happens in both dark and light mode on both browsers.

Note the unnecessary scrollbar on the left sidebar, as well as the right sidebar having two scrollbars (only one of which does anything useful).

Also the real scrollbar appears to be styled incorrectly in the dark theme; see the right sidebar's scrollbar.

👟 Reproduction steps

  1. Generate a basic TechDocs site with 1 or more pages as well as a page with multiple headings.
seq 1 10 | parallel -q mkdir -p 'docs/Sub {}'
seq 1 10 | parallel -q touch 'docs/Sub {}/index.md'
cat << EOF > docs/index.md
# Example Docs page

## H2 - 1
### H3 - 1
#### H4
### H3 - 2
### H3 - 3
### H3 - 4
### H3 - 5
## H2 - 1
### H3 - 1
#### H4
### H3 - 2
### H3 - 3
### H3 - 4
### H3 - 5
### H3 - 6
## H2 - 1
### H3 - 1
#### H4
### H3 - 2
### H3 - 3
### H3 - 4
### H3 - 5
### H3 - 6
## H2 - 1
### H3 - 1
#### H4
### H3 - 2
### H3 - 3
### H3 - 4
### H3 - 5
### H3 - 6
### H3 - 6
EOF
  1. Configure Backstage with what's needed to build and render the site
# catalog-info.yaml
apiVersion: backstage.io/v1alpha1
kind: Component
metadata:
  name: scrollbars-why
  annotations:
    backstage.io/techdocs/ref: dir:.
spec:
  type: service
  owner: guest
  lifecycle: experimental
# app-config.local.yaml
techdocs:
  builder: local
  publisher:
    type: local
  generator:
    runIn: local

📃 Provide the context for the Bug.

I played around a bit but couldn't figure out a proper fix without a lot of extra work since there's no good way to "monkeypatch" the CSS the techdocs plugin injects. But I think the fix is to just set overflow-y: visible; on .md-sidebar__scrollwrap. If you make that change via the browser's devtools it fixes the issue.

As for the scrollbar styling with the dark theme, technically that's a different issue, but unsetting scrollbar-color in .md-sidebar makes it look less bad.

🖥️ Your Environment

Firefox 122.0 and Chrome 121.0.6167.139

$ yarn backstage-cli info
OS:   Darwin 23.3.0 - darwin/arm64
node: v18.19.0
yarn: 3.7.0
cli:  0.25.0 (installed)
backstage:  1.21.1

Dependencies:
  @backstage/app-defaults                                          1.4.6
  @backstage/backend-app-api                                       0.5.9
  @backstage/backend-common                                        0.19.9, 0.20.0
  @backstage/backend-dev-utils                                     0.1.2
  @backstage/backend-openapi-utils                                 0.1.1
  @backstage/backend-plugin-api                                    0.6.8
  @backstage/backend-tasks                                         0.5.13
  @backstage/catalog-client                                        1.5.1
  @backstage/catalog-model                                         1.4.3
  @backstage/cli-common                                            0.1.13
  @backstage/cli-node                                              0.2.1
  @backstage/cli                                                   0.24.0, 0.25.0
  @backstage/config-loader                                         1.6.0
  @backstage/config                                                1.1.1
  @backstage/core-app-api                                          1.11.2
  @backstage/core-compat-api                                       0.1.0
  @backstage/core-components                                       0.13.9
  @backstage/core-plugin-api                                       1.8.1
  @backstage/dev-utils                                             1.0.25
  @backstage/e2e-test-utils                                        0.1.0
  @backstage/errors                                                1.2.3
  @backstage/eslint-plugin                                         0.1.4
  @backstage/frontend-plugin-api                                   0.4.0
  @backstage/integration-aws-node                                  0.1.8
  @backstage/integration-react                                     1.1.22
  @backstage/integration                                           1.8.0
  @backstage/plugin-api-docs                                       0.10.2
  @backstage/plugin-app-backend                                    0.3.56
  @backstage/plugin-app-node                                       0.1.8
  @backstage/plugin-auth-backend-module-atlassian-provider         0.1.0
  @backstage/plugin-auth-backend-module-gcp-iap-provider           0.2.2
  @backstage/plugin-auth-backend-module-github-provider            0.1.5
  @backstage/plugin-auth-backend-module-gitlab-provider            0.1.5
  @backstage/plugin-auth-backend-module-google-provider            0.1.5
  @backstage/plugin-auth-backend-module-oauth2-provider            0.1.5
  @backstage/plugin-auth-backend-module-oauth2-proxy-provider      0.1.0
  @backstage/plugin-auth-backend-module-okta-provider              0.0.1
  @backstage/plugin-auth-backend                                   0.20.2
  @backstage/plugin-auth-node                                      0.4.2
  @backstage/plugin-catalog-backend-module-scaffolder-entity-model 0.1.5
  @backstage/plugin-catalog-backend-module-unprocessed             0.3.5
  @backstage/plugin-catalog-backend                                1.16.0
  @backstage/plugin-catalog-common                                 1.0.19
  @backstage/plugin-catalog-graph                                  0.3.2
  @backstage/plugin-catalog-import                                 0.10.4
  @backstage/plugin-catalog-node                                   1.6.0
  @backstage/plugin-catalog-react                                  1.9.2
  @backstage/plugin-catalog-unprocessed-entities                   0.1.6
  @backstage/plugin-catalog                                        1.16.0
  @backstage/plugin-devtools-backend                               0.2.5
  @backstage/plugin-devtools-common                                0.1.7
  @backstage/plugin-devtools                                       0.1.7
  @backstage/plugin-events-node                                    0.2.17
  @backstage/plugin-github-actions                                 0.6.9
  @backstage/plugin-home-react                                     0.1.6
  @backstage/plugin-org                                            0.6.18
  @backstage/plugin-pagerduty                                      0.7.0
  @backstage/plugin-permission-common                              0.7.11
  @backstage/plugin-permission-node                                0.7.19
  @backstage/plugin-permission-react                               0.4.18
  @backstage/plugin-proxy-backend                                  0.4.6
  @backstage/plugin-scaffolder-backend-module-azure                0.1.0
  @backstage/plugin-scaffolder-backend-module-bitbucket            0.1.0
  @backstage/plugin-scaffolder-backend-module-gerrit               0.1.0
  @backstage/plugin-scaffolder-backend-module-github               0.1.0
  @backstage/plugin-scaffolder-backend-module-gitlab               0.2.11
  @backstage/plugin-scaffolder-backend                             1.19.2
  @backstage/plugin-scaffolder-common                              1.4.4
  @backstage/plugin-scaffolder-node                                0.2.9
  @backstage/plugin-scaffolder-react                               1.7.0
  @backstage/plugin-scaffolder                                     1.17.0
  @backstage/plugin-search-backend-module-catalog                  0.1.12
  @backstage/plugin-search-backend-module-pg                       0.5.17
  @backstage/plugin-search-backend-module-techdocs                 0.1.12
  @backstage/plugin-search-backend-node                            1.2.12
  @backstage/plugin-search-backend                                 1.4.8
  @backstage/plugin-search-common                                  1.2.9
  @backstage/plugin-search-react                                   1.7.4
  @backstage/plugin-search                                         1.4.4
  @backstage/plugin-tech-radar                                     0.6.11
  @backstage/plugin-techdocs-backend                               1.9.1
  @backstage/plugin-techdocs-module-addons-contrib                 1.1.3
  @backstage/plugin-techdocs-node                                  1.11.0
  @backstage/plugin-techdocs-react                                 1.1.14
  @backstage/plugin-techdocs                                       1.9.2
  @backstage/plugin-user-settings                                  0.7.14
  @backstage/release-manifests                                     0.0.11
  @backstage/test-utils                                            1.4.6
  @backstage/theme                                                 0.4.4, 0.5.0
  @backstage/types                                                 1.1.1
  @backstage/version-bridge                                        1.0.7

👀 Have you spent some time to check if this bug has been raised before?

🏢 Have you read the Code of Conduct?

Are you willing to submit PR?

Yes I am willing to submit a PR!

GLundh commented 9 months ago

The issue is visible on the demo site: https://demo.backstage.io/catalog/default/component/backstage/docs

GLundh commented 9 months ago

Here is my closed issue from -22: https://github.com/backstage/backstage/issues/13717

jenreiher-bcgov commented 9 months ago

@sapslaj thanks for filing this! our team would love for this to be improved for better usability in techdocs. We'd love to see this fixed upstream from us, and are happy to support this if we can be at all helpful!

GLundh commented 9 months ago

A work-around for the double scrollbars:

packages/app/src/App.css:

/* TechDoc overflow fix */
#root > div > main > article > div > main {
  overflow-y: visible;
  height: 100%;
}

/* TechDoc padding fix */
#root > div > main > article > div > main > article,
#root > div > main > article > div > main > div {
  padding: 0;
}
axdotl commented 8 months ago

Would be great if this could be fixed, even in the Demo Instance there are not only redundant scrollbars but also scrolling works not reliable in the left navigation pane.

dotboris commented 8 months ago

I might have a hint on where this issue is coming from for the doc navigation and ToC. There's a negative bottom margin on the nav for those. When I disable it in the dev console the problem goes away (Firefox on Mac)

image

I have no idea why that negative margin is there tho.

axdotl commented 8 months ago

Let's see, whether the change done in the following PR fix the issue:

techcommdude commented 8 months ago

I'm experiencing this with Chrome and Edge, so it would be nice to have this fixed. Here's an example with Edge:

Edge-issue

ElementalWarrior commented 7 months ago

This is caused by this line in the page component: https://github.com/backstage/backstage/blob/4935ebd06fca77679629b289b9ac511c5191a25e/packages/core-components/src/layout/Page/Page.tsx#L31

Which is called by the TechdocsReaderPage: https://github.com/backstage/backstage/blob/4935ebd06fca77679629b289b9ac511c5191a25e/plugins/techdocs/src/reader/components/TechDocsReaderPage/TechDocsReaderPage.tsx#L196

Perhaps this should be changed to minheight? That fixes it if I change it in devtools.

github-actions[bot] commented 5 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

jenreiher-bcgov commented 5 months ago

Just bumping this as it's still being reported repeatedly by our users as frustrating!

zjpersc commented 5 months ago

I concur. We're just beginning our TechDocs rollout and we're already seeing this behavior. Consumer feedback has not been positive with the duplicative scrollbars.

GLundh commented 5 months ago

While a real fix is needed, to make your users happy, you can work around it by adding the following to your App.css:

/* TechDoc overflow fix */
#root > div > main > article > div > main {
  overflow-y: visible;
  height: 100%;
}

/* TechDoc padding fix */
#root > div > main > article > div > main > article,
#root > div > main > article > div > main > div {
  padding: 0;
}
farn-dawg commented 5 months ago

While a real fix is needed, to make your users happy, you can work around it by adding the following to your App.css:

/* TechDoc overflow fix */
#root > div > main > article > div > main {
  overflow-y: visible;
  height: 100%;
}

/* TechDoc padding fix */
#root > div > main > article > div > main > article,
#root > div > main > article > div > main > div {
  padding: 0;
}

@GLundh, We created an App.css and used this code. It didn't seem to fix the problem. Are we missing something?

GLundh commented 5 months ago

While a real fix is needed, to make your users happy, you can work around it by adding the following to your App.css:

/* TechDoc overflow fix */
#root > div > main > article > div > main {
  overflow-y: visible;
  height: 100%;
}

/* TechDoc padding fix */
#root > div > main > article > div > main > article,
#root > div > main > article > div > main > div {
  padding: 0;
}

@GLundh, We created an App.css and used this code. It didn't seem to fix the problem. Are we missing something?

packages/app/src/index.tsx:

import React from 'react';
import ReactDOM from 'react-dom/client';
import App from './App';
import './App.css';

ReactDOM.createRoot(document.getElementById('root')!).render(<App />);
awanlin commented 5 months ago

Re-opening this as this hasn't be resolved based on recent comments. I'm reaching out to the TechDocs team as well.

@GLundh, just curious, you've got a fix you are suggesting for others, is there any reason you haven't contributed that back as a PR? Or is there some edge cases that this doesn't cover?

GLundh commented 5 months ago

Fixing the issue with global app css rules is just a hacky workaround. I don't understand the techdocs stack enough to write a fix that I'm happy with. This is just about good enough to make our users happy (and I kinda understand others' pain of trying to roll out techdocs with this bug). I have no idea if it works well enough with other themes.

I still hope that someone that really gets the techdocs-stuff takes a look at the issue.

awanlin commented 5 months ago

Thanks @GLundh, appreciate the follow up. My CSS skills are not enough to take this on sadly.

farn-dawg commented 5 months ago

While a real fix is needed, to make your users happy, you can work around it by adding the following to your App.css:

/* TechDoc overflow fix */
#root > div > main > article > div > main {
  overflow-y: visible;
  height: 100%;
}

/* TechDoc padding fix */
#root > div > main > article > div > main > article,
#root > div > main > article > div > main > div {
  padding: 0;
}

@GLundh, We created an App.css and used this code. It didn't seem to fix the problem. Are we missing something?

packages/app/src/index.tsx:

import React from 'react';
import ReactDOM from 'react-dom/client';
import App from './App';
import './App.css';

ReactDOM.createRoot(document.getElementById('root')!).render(<App />);

@GLundh Thank you. Unfortunately it didn't work for us. No idea why. This seems like a fundamental problem in TechDocs. Would be nice to see a permanent fix.

freben commented 5 months ago

Alright, see PR above, making things a whole lot better.

vaniri commented 3 weeks ago

On the demo site I noticed that while scrolling TOC sidebar doesn't highlight the active heading. I also have this problem in my project. Does anyone know what might be the cause?