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

Don't stack PR tab menu on small screens #25789

Merged
merged 16 commits into from
Jul 14, 2023

Conversation

sebastian-sauer
Copy link
Contributor

@sebastian-sauer sebastian-sauer commented Jul 9, 2023

the stacking takes up screen space - display the tabs as the navigation bar. github uses the same layout.

Screenshots (left before, right after):

image image

Large screen:

image

the stacking takes up screen space - display the tabs as the navigation bar.
github uses the same layout.
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 9, 2023
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 9, 2023
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 9, 2023
Copy link
Member

@denyskon denyskon left a comment

Choose a reason for hiding this comment

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

Could you please do it in the CSS file?

margin-bottom: 1rem;

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 9, 2023
@sebastian-sauer
Copy link
Contributor Author

Could you please do it in the CSS file?

margin-bottom: 1rem;

sorry - not sure what I should do in the CSS file?

@denyskon
Copy link
Member

denyskon commented Jul 9, 2023

Please set

overflow-x: auto;
overflow-y: hidden;

inside the existing CSS class for .pull.tabular.menu instead of adding helper classes. They are a good idea if there is no existing class for an element or if it is using a generalized class, but if there is already a class for the element, it is cleaner to use it.

@wxiaoguang
Copy link
Contributor

I am afraid this change is not right on desktop view.

IIRC the "bottom line" of the menu would be broken

@techknowlogick techknowlogick added the topic/ui Change the appearance of the Gitea UI label Jul 10, 2023
@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 10, 2023
@sebastian-sauer
Copy link
Contributor Author

sebastian-sauer commented Jul 10, 2023

I am afraid this change is not right on desktop view.

IIRC the "bottom line" of the menu would be broken

Good catch - i've now added a divider below and fixed layout for large screens.

Screenshots in PR description are updated.

diff ignoring whitespace change:
https://github.com/go-gitea/gitea/pull/25789/files?diff=unified&w=1

@denyskon denyskon self-requested a review July 10, 2023 20:32
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Jul 10, 2023
Copy link
Member

@denyskon denyskon left a comment

Choose a reason for hiding this comment

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

Looks good now in both mobile and desktop view 👍

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 10, 2023
@denyskon denyskon added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 10, 2023
@denyskon denyskon added this to the 1.21.0 milestone Jul 10, 2023
@lunny lunny enabled auto-merge (squash) July 14, 2023 01:28
@lunny lunny merged commit b81c013 into go-gitea:main Jul 14, 2023
23 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 14, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 14, 2023
* giteaofficial/main:
  Fix incorrect release count (go-gitea#25879)
  Add Github related extensions in devcontainer (go-gitea#25800)
  Add error info if no user can fork the repo (go-gitea#25820)
  Fix wrong usage of PathEscapeSegments in branch list page (go-gitea#25864)
  fix incorrect repo url when changed the case of ownername (go-gitea#25733)
  Upgrade go dependencies (go-gitea#25819)
  Don't stack PR tab menu on small screens (go-gitea#25789)
  Link to list of vulnerabilities (go-gitea#25872)
  [skip ci] Updated translations via Crowdin
  move issue filters to shared template (go-gitea#25729)
  [skip ci] Updated translations via Crowdin
  Remove `git.FileBlame` (go-gitea#25841)
  Fix empty project displayed in issue sidebar (go-gitea#25802)
  Update blog links (go-gitea#25843)
  Fix margin on the `new/edit milestone` page (go-gitea#25801)
  Do not "guess" the file encoding/BOM when using API to upload files (go-gitea#25828)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants