Fix misaligned tab list indentation on mobile

[pat-s]

Remove the unneeded and unresponsive chevron-right in mobile view and instead use a plain list in which the active item is underlined.

fix #2162 (comment)

[pat-s]

I suspect some windi issues here. Let's wait until #4614 is done

[pat-s]

OK got it working now.

[xoxys]

@pat-s Can we apply the formatting changes in a single PR that does not contain anything else? Its really hard to review such PRs.

[pat-s]

Yes, I know, I did #4622 to solve this.
But this is a bit blocked / WIP still.
We can push #4622 first, then the unrelated sorting changes should be wiped in the other ones.

[xoxys]

Why not just disable your plugin for now. I'm ok to review the existing PRs in this state, but would reject any new PR with such an amount of unrelated changes.

[xoxys]

px-2 got lost here:

[xoxys]

Personally, I don't think this way of highlighting looks nice. What do you think about using the same way we use for pipeline list items and use the hover bg?

[pat-s]

Why not just disable your plugin for now.

Yes, I should have done so already. Just didn't think about it and went on doing work.

[pat-s]

What do you think about using the same way we use for pipeline list items and use the hover bg?

Is an option. I tried it but didn't like that the line highlights spawns so wide to the right. For steps it is smaller and hence less an issue. But I can try, makes sense to use the same approach.

[xoxys]

Yes, but not using the full width, it is also bad on mobile.

[xoxys]

w-full and w-auto should not be used together and as I said already, I'm strictly against not using full width on mobile.

[pat-s]

Trying this out, I think now that using full-width on mobils is a mistake.
It forces the tabs to be sorted below each other, by this using up a lot of vertical screen space and leaving a lot unused. In addition, a full width background highlighting of the active one looks really odd.

By restricting width, the tabs are aligned horizontally. To me, this makes much better use of the available space.

This is a mobile screenshot:

[xoxys]

Sorry but then at least I will not approve it. It looks ugly, is pretty uncommon, and its difficult to hit the correct button on small screens:

[pat-s]

Can you name an argument why the vertical variant would be better? I can't find any.

The colored space is fully unused for no reason. It covers 60% of the default horizontal screen space on mobile.

[xoxys]

Im not blind :) Im aware of the spacing issue and yes we can improve the menu with something like a "more" menu on mobile e.g. similar to gh:

But that's not that easy and needs more work and is not a topic for this PR. I added my argumentation to my last comment.

[xoxys]

I don't like that we mix in more and more issues and new ideas in existing PRs. If you want to get forward, I would suggest staying with the current layout. Otherwise, it needs to wait until it gets more feedback/approvals from other maintainers. I cant approve it as I think its technically and in terms of UX wrong.

[pat-s]

I see your point and I am also fine to revert to keep this PR small. However, it sometimes/often happens that one stumbles over an issue only when iterating on a specific topic. Then again it might happen that the proposed solution doesn't work so well and including an additional change might round up the picture of the PR.

TBH, I am absolutely unhappy with adding a full-width bg highlighting. In that case, I would have a strong vote in favor of the initially proposed underline.
To find a middleground that suits both, I've started iterating on a "good" bg highlighting approach that would suit both. Yet now we're here in a semi-complicated (meta) discussion.

One option now is to merge the "small" PR variant with the full width bg highlighting but I will open add a follow-up PR then to propose a change to a horizontal layout (which hopefully doesn't get denied then as otherwise I would feel somewhat played out 😅️).
If that is not agreed on, I'd favor to not merge this PR and let it be evaluated again later in a broader scope, once I've finished to come up with such a horizontal solution.

[xoxys]

This is not a meta discussion. You changed the overall layout fundamentally in the middle of a PR that aims to fix a small alignment issue. I will for sure deny any horizontal layout on mobile 😅 We can make the mobile nav a rollup menu or a popup like github or gitea do or any similar approach but is has to be vertically aligned in any case.

As you like to compare to other well known apps as reference, can you name a single one where the mobile nav looks like the one you proposed? I cant and there is a good reason for it.

Im ok to go with an underline indicator for now, even if the bg indicator isnt that bad...

[pat-s]

This is not a meta discussion. You changed the overall layout fundamentally in the middle of a PR that aims to fix a small alignment issue.

Yes, but I didn't hide this change and fully OK in general with the idea of changing a PR scope after creation for a good reason. If this would have been the initial scope, nothing would be wrong about it.
And I'd rather have a PR that ends up in a "working" solution than a partially working one, even if that means extending/delaying the PR. It is not about winning a sprint in merging PRs.
I also think it is pretty normal to discover such things when you touch parts of the code base for the first time (personally) and then find out that XYZ are suboptimal but should ideally be changed together.

I started with wanting to fix the indentation of the chevron-right icon. Then I realized it has no function at all and only exists on mobile and should likely be removed. While at it, I realized there is no indicator which tab is actually active, so I had the idea of the underline. You then brought up the bg highlighting idea, which is good and I tried to add. I then (accidentally) discovered the forced full-width for each of these items and the odd look of the full-width highlight and now we're here.
(I know you're aware of all this, just wanted to outline how the overall process was and that there's always a reason for every step in the process).

We can make the mobile nav a rollup menu or a popup like github or gitea do or any similar approach but is has to be vertically aligned in any case.

I am with you for the admin list as this is one is long and might potentially get even longer. In such cases I am fully with you and would even say that there should be hamburger fold by default (as for GH and GL mobile).

The pipeline list however consists of three fixed items which perfectly fit horizontally without needing folding. I don't see why these must be aligned vertically at all costs with a questionable full-width highlighting. Have you tested the current state directly on your phone? It is much better than before (again, only talking about the pipeline list). We don't need hover on mobile (this still needs to be removed).

---

PS: Thanks for the calm and positive-minded discussion. I think such discussion are required to achieve (good) improvements in the end. Appreciate it 🤝️

[xoxys]

The pipeline list however consists of three fixed items which perfectly fit horizontally without needing folding.

The menu should work in the same way for all pages. Just because it looks ok on one page doesnt solve the problem. Besides that what happens if more menu items are added? Or someone uses a smaller phone than yours? The approach chosen by GH keeps items vertically aligned as long as they fit in one line, all items that dont fit in this single line are moved into a "more" menu dynamically. As I said, Im fine with such an approach.

We don't need hover on mobile (this still needs to be removed).

Well hover effects apply e.g. on long press actions on mobile. I dont see a reason to remove them, in best case they never appear and can just stay instead of adding more code to remove them.

[xoxys]

border-radius is missing and it is the wrong color.

[woodpecker-bot]

Tearing down https://woodpecker-ci-woodpecker-pr-4610.surge.sh