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

Logo Overlaps with Menu in RTL Languages #45077

Open
mboukhalfa opened this issue Feb 9, 2024 · 10 comments
Open

Logo Overlaps with Menu in RTL Languages #45077

mboukhalfa opened this issue Feb 9, 2024 · 10 comments
Labels
area/localization General issues or PRs related to localization area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@mboukhalfa
Copy link
Member

mboukhalfa commented Feb 9, 2024

This is a Bug Report

Problem:
When switching to an RTL language, the logo overlaps with the menu, creating a layout issue as shown in the picture! This is different form the issue google/docsy#1442 and google/docsy#829 these issues are discussing switching the positioning for of menu elements to fit with RTL but the logo overlap problem is not observed in the docsy check: https://example.docsy.dev/fa/

The root cause of the problem is suspected to be the custom CSS applied in the k8s website. Here

.navbar-brand {
position: absolute;
width: 45px;
height: 44px;
background-repeat: no-repeat;
background-size: contain;
background-image: url("/images/favicon.png");
}

Screenshot from 2024-02-07 15-31-27
Proposed Solution:
To fix this we can override the logo position with RTL languages for example:

body:lang(fa),
body:lang(ar),
body:lang(az),
body:lang(dv),
body:lang(he),
body:lang(ku),
body:lang(ur) {
  .navbar-brand {
    left: 16px;
  }
}

However, it's recommended to consider a more comprehensive solution. Refactoring the custom CSS added to the k8s website. We have big custom css (>1000 lines ) overriding the default docsy! might be necessary to avoid interfering with the default docsy styling. It's advisable to minimize overrides to essential elements, aligning with the purpose of using docsy. This approach ensures a cleaner separation between documentation content and web development concerns.
Page to Update:
https://deploy-preview-45056--kubernetes-io-main-staging.netlify.app/ar/

@mboukhalfa mboukhalfa added the kind/bug Categorizes issue or PR as related to a bug. label Feb 9, 2024
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Feb 9, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

SIG Docs takes a lead on issue triage for this website, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dipesh-rawat
Copy link
Member

/area web-development

@k8s-ci-robot k8s-ci-robot added the area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes label Feb 10, 2024
@Gauravpadam
Copy link
Member

@mboukhalfa is it an issue with all RTL languages?

@Gauravpadam
Copy link
Member

In that case, we'll need a more flexible solution than just changing the css positions; I've got something in mind

@mboukhalfa
Copy link
Member Author

@mboukhalfa is it an issue with all RTL languages?

yes it is

@mboukhalfa
Copy link
Member Author

In that case, we'll need a more flexible solution than just changing the css positions; I've got something in mind

Can you elaborate what a flexible solution would be ?

@sftim
Copy link
Contributor

sftim commented Feb 10, 2024

/area localization

/remove-kind bug
We don't have RTL support yet, so it's more a feature request (see #22730 for the overall feature request).

@k8s-ci-robot k8s-ci-robot added area/localization General issues or PRs related to localization needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Feb 10, 2024
@sftim
Copy link
Contributor

sftim commented Feb 10, 2024

We have big custom css (>1000 lines ) overriding the default docsy! might be necessary to avoid interfering with the default docsy styling.

See #41171 for an issue about reducing our level of customization. It's useful work that we have not yet staffed.

@chalin
Copy link
Contributor

chalin commented Apr 24, 2024

Hi all: if you could upvote google/docsy#1442 that would help it get some traction.

@mboukhalfa
Copy link
Member Author

mboukhalfa commented Apr 28, 2024

Hmm I think I found the issue it seems we are using ml-md-auto class to push the menu always to the right side however this classes are not supporting direction change as show in the issue so when we switch to RTL the logo go to the other side but the menu keep its place instead of moving to left side

<div class="td-navbar-nav-scroll ml-md-auto" id="main_navbar">

twbs/bootstrap#22780 (comment)

Probably using d-flex flex-row-reverse in the nav instead of flex-column flex-md-row can help having the same effect but then we need to deal with the logo differently there is a room for improvement there I think it is not a good idea to include the logo with css as a background to some element ! we can just follow the theme and add few customization to have the current rendering

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/localization General issues or PRs related to localization area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests

6 participants