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

Two row layout for title and context of pipeline list #4625

Merged
merged 7 commits into from
Dec 30, 2024

Conversation

qwerty287
Copy link
Contributor

@qwerty287 qwerty287 commented Dec 27, 2024

supersedes #3976

Before:
image

After:
image

@qwerty287 qwerty287 added ui frontend related enhancement improve existing features labels Dec 27, 2024
@qwerty287 qwerty287 requested a review from a team December 27, 2024 10:54
@pat-s
Copy link
Contributor

pat-s commented Dec 27, 2024

Can you add a screenshot?

@xoxys
Copy link
Member

xoxys commented Dec 27, 2024

Added one to the PR description.

Copy link
Member

@xoxys xoxys left a comment

Choose a reason for hiding this comment

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

Im really missing the #<pipeline number> information. Can we add it back?

@qwerty287
Copy link
Contributor Author

Screenshot 2024-12-27 at 17-52-53 Woodpecker

@pat-s
Copy link
Contributor

pat-s commented Dec 27, 2024

Would it be better to have it in the second line? This line already holds other static metadata while the first one holds the title information.

@xoxys
Copy link
Member

xoxys commented Dec 27, 2024

Fine for me as well even if I like it more in the title. If we add it to the second line I would add as the first element and we need to add an pound icon instead of the text #.

@qwerty287
Copy link
Contributor Author

Can do that, but then I need you CSS help.

The problem is that I'd like to have it in a grid to have icons under icons and text under text. That works for the commit hash because it's always 8 chars, so the width can be fixed, but the pipeline number can be endless. Is there a way to "sync" the width of the ID field over all pipelines in the list?

@xoxys
Copy link
Member

xoxys commented Dec 27, 2024

Not sure if I can follow. But IMO this is getting way too complicated now. Either keep it in the title as it was before or leave it out and improve it in a dedicated PR.

@qwerty287
Copy link
Contributor Author

I want to have a consistency in all the second lines. I.e. all pipeline numbers, all commit hashs, all contexts (branch, pr) in one line.
I don't know how to do that with CSS if you can't use a fixed size.

But I also think first line is fine

@xoxys
Copy link
Member

xoxys commented Dec 27, 2024

Ok got it, thanks. I can look into this in another PR if you like.

@xoxys
Copy link
Member

xoxys commented Dec 27, 2024

I have looked into this and I am sure that this will not work, at least not with pure CSS.

@xoxys
Copy link
Member

xoxys commented Dec 27, 2024

@pat-s @anbraten Are you ok to keep it in the title for now until we find a way to add it to the second line? Otherwise we might want to remove it again from this PR to get forward.

@pat-s
Copy link
Contributor

pat-s commented Dec 27, 2024

Temporary OK, if you commit to look into this in more detail. The question is just what happens if this is "not so easy" or gets stuck - then we have a complicated discussion.

Maybe the motivation would be higher for a follow-up PR if the workaround wouldn't be included? 🙈️

Your choice, I'm either way.

@xoxys
Copy link
Member

xoxys commented Dec 27, 2024

Just dont like that we loose a functionality here. To be honest I dont know how to solve the issue with the second line. This is not only an issue with the pipeline number. With the current approach we will get this problem with every other information that can have a dynamic length.

@xoxys
Copy link
Member

xoxys commented Dec 27, 2024

The other alternative it to accept that we will get a similar look like this with the second line:

image

@qwerty287 could we live with that? I know its not optimal but I dont even see a way with javascript without adding a lot of complexity.

@qwerty287
Copy link
Contributor Author

I think on github it looks better, maybe because you can really see the different widths. In the pipeline list these are very small differences, so not directly visible, but you still see that something is wrong and unaligned there.

@xoxys
Copy link
Member

xoxys commented Dec 28, 2024

Ok then I have no idea how to proceed.

@pat-s pat-s merged commit 51dd42c into woodpecker-ci:main Dec 30, 2024
7 checks passed
@woodpecker-bot
Copy link
Collaborator

@woodpecker-bot woodpecker-bot mentioned this pull request Dec 30, 2024
1 task
@qwerty287 qwerty287 deleted the arrange-pipeline-list branch December 30, 2024 10:11
pat-s added a commit that referenced this pull request Dec 30, 2024
pat-s added a commit that referenced this pull request Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing features ui frontend related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants