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 excessive vote column width #3881

Open
wants to merge 1 commit into
base: master-7
Choose a base branch
from
Open

Conversation

asdil12
Copy link

@asdil12 asdil12 commented Feb 25, 2025

Only make the columns as wide as needed.
Allow them to grow if there is unused space left.
Do not allow them to grow and then show a horizontal scrollbar.

Before:
image

After:
image

Horizontal scrolling still works if there is really not enough space:
image

Only make the columns as wide as needed.
Allow them to grow if there is unused space left.
Do not allow them to grow and then show a horizontal scrollbar.

Signed-off-by: Dominik Heidler <[email protected]>
Copy link
Collaborator

@dartcafe dartcafe left a comment

Choose a reason for hiding this comment

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

Please add tests containing more variations:

  • date polls with date and time ranges
  • a mixture of them
  • Text polls

@dartcafe dartcafe added this to the backlog milestone Feb 25, 2025
@asdil12
Copy link
Author

asdil12 commented Feb 25, 2025

For text polls:

Old:
10

New:
11

New if we would additionally decrease max-width from 19em to 11em:
12

@asdil12
Copy link
Author

asdil12 commented Feb 25, 2025

Date polls:

Old:
13

New:
14

@asdil12
Copy link
Author

asdil12 commented Feb 25, 2025

Not sure what you mean with a mixture of them.
It doesn't seem possible to have both text and date options in a single poll.

@dartcafe
Copy link
Collaborator

Not sure what you mean with a mixture of them.

I meant what is displayed with the last screenshot.

But your screenshots display the reason, why the width is set. With different widths of the header content also the columns will have different widths.

This should be avoided from an UX perspective, since the different width may signal different weights and importance of the options.

So the goal was (and should be kept) to have identical widths of the columns. I have to admit that the current solution was a shortcut to achieve that. But with an auto setting, we still have the initial situation.

If you have a solution, to ensure, that the width is set even over all columns by saving space, we should try it.

@asdil12
Copy link
Author

asdil12 commented Feb 25, 2025

Even with the current css the width may vary from 11em to 19em on each column depending on the content. You would see it especially on polls with few options on large screens.

A possible solution would be to only do this solution for date polls, which don't vary that much in column title width.

But in general I much prefer showing everything on one screen without scrolling over ensuring even column width. It is very painful to not see the overview which is important for the task it tries to achieve. Especially horizontal scrolling should be avoided at all cost.

@pheerai
Copy link

pheerai commented Mar 5, 2025

In general, the requirement for "All option columns should have the same width" somewhat indicates to me that a grid would be a better fit compared to flex.

The following seems to work fine for me:

  • In .vote-table__users: flex: 0 fit-content
  • In .vote-table__votes: display: grid instead of flex and grid-template-columns: repeat(999, max-content)

Are there any edge-cases that break through the max-content here that I'm not seeing?

Of course, the hardcoded 999 needs to be replaced with the actual number of options, but I'm not well-versed enough with Vue to know whether that's something one would do (and, if so, how exactly).

Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

@dartcafe
Copy link
Collaborator

@pheerai Yeah. From time to time, I was thinking about changing it to a grid view, but had not the mood or time to get into it, since I did not used grid views so far.

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

Successfully merging this pull request may close these issues.

3 participants