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 damaged UI userlist #37477

Conversation

inet-cologne
Copy link

@inet-cologne inet-cologne commented Mar 30, 2023

Summary

Fix CSS for user list backend to dont overlap entries - from grid-auto-rows:minmax(60px, max-content) to grid-auto-rows:minmax(max-content, max-content).

Checklist

@szaimen szaimen added this to the Nextcloud 27 milestone Mar 30, 2023
@szaimen szaimen added bug 3. to review Waiting for reviews labels Mar 30, 2023
@szaimen szaimen requested review from JuliaKirschenheuter, ChristophWurst, a team, artonge, skjnldsv and Pytal and removed request for a team March 30, 2023 07:48
@ChristophWurst ChristophWurst removed their request for review March 30, 2023 07:54
@ChristophWurst
Copy link
Member

@szaimen if you triage a PR and request a review from engineers please make sure the PR is in a reviewable state. To judge the changes here we'd need a description or a linked ticket and before/after screenshots for the visual change. Cheers.

@inet-cologne
Copy link
Author

inet-cologne commented Apr 1, 2023

Hi @szaimen and @ChristophWurst,
thanks a lot for your time to try to review my PR.
I am sorry for the confusion but I am totally new in Github! (Gitlab is so much more intuitive! 😬)

Here are the requested and hopefully sufficient infos for you to acknowledge the changes:
I changed the CSS in that way that the line height dynamically growths dependent of the content.

Bug report reference:
#37188

Screenshots before and after the changes as attachment.

Please let me know if something is missing!

Best,
Chris
Before (bug):
Nextcloud-25-0-5_Error-Overlapping-Userlist_Screenshot_2023-03-31_annotated

After (fix):
Nextcloud-25-0-5_Fix-Userlist_Screenshot_2023-03-31_annotated

@JuliaKirschenheuter JuliaKirschenheuter changed the title Fixed CSS for user list backend to dont overlap entries - from grid-a… Fix damaged UI userlist Apr 3, 2023
Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

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

Thank you @inet-cologne for fixing this issue!
Not tested but looks good!

@skjnldsv

This comment was marked as resolved.

…uto-rows:minmax(60px, max-content) to grid-auto-rows:minmax(max-content, max-content).

Signed-off-by: Chris <inet.cologne@gmail.com>
@nextcloud-command nextcloud-command force-pushed the bugfix/37188/fix-backend-overlapping-userlist branch from a9c171f to 9f330d8 Compare April 4, 2023 07:01
@skjnldsv
Copy link
Member

skjnldsv commented Apr 4, 2023

/compile amend /

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 4, 2023
@skjnldsv
Copy link
Member

skjnldsv commented Apr 4, 2023

@inet-cologne please compile and ship changed dist files :)

@szaimen
Copy link
Contributor

szaimen commented Apr 17, 2023

So basically please rebase, run npm ci and npm run bild and commit the changed files here. Thanks in advance! :)

@szaimen szaimen added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Apr 26, 2023
This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@joshtrichards
Copy link
Member

/rebase

@inet-cologne
Copy link
Author

Hi there,
sorry, I need some help here.
I only changed one small CSS userlist setting in an earlier Nextcloud version from
minmax(60px, max-content)
to
minmax(max-content, max-content

Unfortunately the change was not merged in the follower versions of Nextcloud and the rebase produced now a merge conflict.
I don't know exactly what do do here to solve the merge conflict as my change was only a tiny setting, not the whole class.

Everything was documented in the related PR. Perhaps someone can assist in resolving the conflict please.
Thanks in advance as unfortunately the error is back in the actual Nextcloud versions 27.0.0 to 27.0.2.
Chris

@inet-cologne
Copy link
Author

inet-cologne commented Sep 29, 2023

As said above.
I need somebody to do that stuff here, as I don't know how to get all the Github stuff done.
So please can someone of your experts repair the above "60px" to "max-content" css in the current release in a way that it remains in future versions?
I give up! I'm sorry!
Chris

@ChristophWurst ChristophWurst added 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed 2. developing Work in progress labels Sep 29, 2023
@ChristophWurst ChristophWurst marked this pull request as draft September 29, 2023 08:32
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv skjnldsv added 2. developing Work in progress and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Feb 21, 2024
This was referenced Mar 12, 2024
@Altahrim Altahrim mentioned this pull request Mar 20, 2024
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
@skjnldsv skjnldsv modified the milestones: Nextcloud 30, Nextcloud 31 Aug 14, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 31 milestone Aug 14, 2024
@skjnldsv
Copy link
Member

Hey @inet-cologne after seeing the few issues we had with the user list, it received a refresh which makes your fix not required anymore! 😉

Thanks for taking the time to create this PR and help Nextcloud, and my apologies for taking this long to update this ticket 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants