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

Fix dashboard switching on Mobile #20238

Merged
merged 14 commits into from
Jul 28, 2022
Merged

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented Jul 5, 2022

  • This is a regression of improving mobile experience on Gitea, currently organization dashboard aren't readable and the popup won't show up when you want to switch between users/organization(as we saw in Don't prevent overflow on y-as #19978).
  • This patch fixes that, by allowing the popup to allocate the required pixels(for some absurd reason, z-index doesn't work on the popup, so it's not able to render over the existing elements, we can investigate later of why this is). And also remove the additional dropdown menu for the pages link, so it's one unified list which then can be displayed as rows.

Before

After

- This is a regression of improving mobile experience on Gitea,
currently organization dashboard aren't readable and the popup won't
show up when you want to switch between users/organization(as we saw in
- This patch fixes that, by allowing the popup to allocate the required
pixels(for some absurd reason, z-index doesn't work on the popup, so
it's not able to render over the existing elements, we can investigate
later of why this is). And also remove the additional dropdown menu for
the pages link, so it's one unified list which then can be displayed as rows.
@Gusted Gusted added this to the 1.18.0 milestone Jul 5, 2022
@Gusted Gusted added type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/mobile backport/v1.17 labels Jul 5, 2022
Gusted pushed a commit to Gusted/gitea that referenced this pull request Jul 5, 2022
- Backport go-gitea#20238
  - This is a regression of improving mobile experience on Gitea, currently organization dashboard aren't readable and the popup won't show up when you want to switch between users/organization(as we saw in go-gitea#19978).
  - This patch fixes that, by allowing the popup to allocate the required pixels(for some absurd reason, z-index doesn't work on the popup, so it's not able to render over the existing elements, we can investigate later of why this is). And also remove the additional dropdown menu for the pages link, so it's one unified list which then can be displayed as rows.
  - See original PR for screenshots.
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 5, 2022
web_src/less/_base.less Outdated Show resolved Hide resolved
@zeripath
Copy link
Contributor

zeripath commented Jul 5, 2022

Hmm...

Interesting diff, you've removed the "right stackable menu" wrapping div from around the organisations action menu and then added an !important selector to stackable? These don't seem intimately related to fixing the context selector drop-down and your screenshots don't appear to be showing what happens to this action menu.

@Gusted
Copy link
Contributor Author

Gusted commented Jul 5, 2022

  • And also remove the additional dropdown menu for the pages link, so it's one unified list which then can be displayed as rows.

If we don't have this change(requires to have that removed div to have the ui class):

image

Or we drop the ui requrement from the general fix .ui.stackable.menu:not(.no-vertical-tabs) but then it's forced to have !important on the flex-direction as fomantic can take precedence otherwise.

@Gusted
Copy link
Contributor Author

Gusted commented Jul 6, 2022

Removed the for now unnecessary !important.

@GiteaBot GiteaBot removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 10, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jul 10, 2022
@zeripath
Copy link
Contributor

I think we need to do a similar thing to the navbar trick but this is a definite improvement.

There's an issue with @mediaMd spilling over now though.

@6543
Copy link
Member

6543 commented Jul 24, 2022

what's the current status ?!?

@Gusted
Copy link
Contributor Author

Gusted commented Jul 25, 2022

what's the current status ?!?

Waiting on answer from @zeripath, but I can go ahead and implement my mentioned fix. However #20393 was merged. So we might re-use that logic for the dropdown menu.

@zeripath
Copy link
Contributor

Sorry @Gusted didn't realise you were waiting on me - Do you have an updated PR that you were thinking on showing me?

@Gusted
Copy link
Contributor Author

Gusted commented Jul 26, 2022

Do you have an updated PR that you were thinking on showing me?

Currently not. Because I don't have a clue how to proceed. I could see if tippy.js would work for the dropdowns and have a general fix. Or just fiddle around with CSS and HTML structures so the original colors won't be lost, as noted in your review.

@Gusted
Copy link
Contributor Author

Gusted commented Jul 28, 2022

Ugh dropdown seems to be bit more complicated to make it compatible with tippy.js. So for now just dirty CSS to fix this issue.

@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 28, 2022
6543 pushed a commit that referenced this pull request Jul 28, 2022
- This is a regression of improving mobile experience on Gitea, currently organization dashboard aren't readable and the popup won't show up when you want to switch between users/organization(as we saw in #19978). 
- This patch fixes that, by allowing the popup to allocate the required pixels(for some absurd reason, z-index doesn't work on the popup, so it's not able to render over the existing elements, we can investigate later of why this is). And also remove the additional dropdown menu for the pages link, so it's one unified list which then can be displayed as rows.
@6543
Copy link
Member

6543 commented Jul 28, 2022

🪨

@6543 6543 merged commit 9691d43 into go-gitea:main Jul 28, 2022
@6543 6543 added the backport/done All backports for this PR have been created label Jul 29, 2022
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
- This is a regression of improving mobile experience on Gitea, currently organization dashboard aren't readable and the popup won't show up when you want to switch between users/organization(as we saw in go-gitea#19978). 
- This patch fixes that, by allowing the popup to allocate the required pixels(for some absurd reason, z-index doesn't work on the popup, so it's not able to render over the existing elements, we can investigate later of why this is). And also remove the additional dropdown menu for the pages link, so it's one unified list which then can be displayed as rows.
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/mobile type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants