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 time menu click area #4125

Merged
merged 10 commits into from
Nov 18, 2024
Merged

Conversation

vkrasnovyd
Copy link
Contributor

Resolves #4009

Class 'icon-less-space-to-left' reduced the menu opening dots area width in order to fit all the times into the widget.

This commit restores this area width and reduces its right margin to make the click area overlap the dots correctly without increasing the time block width.

@bastianjoel bastianjoel assigned MSoeb and unassigned bastianjoel Sep 16, 2024
@vkrasnovyd
Copy link
Contributor Author

This pull request resolves issue when the click area and the hover shadow were not overlaping the three dots. However, the overall layout may need some changes to look neater. I suggest discussing them and writing a new wanted behaviour here before reworking this area.

Here are some issues with the current layout and my suggestions how to resolve it:

Position of the menu dots

It's part B of the expected behavior in the original issue #4009: the menu being too far avay from the time.
According to @rrenkert this can't be achieved with the current style rules:

  • the button should remain square and its shadow should be circle for maintaining consistent design
  • the button shouldn't be moved to the left. Othervise the shadow would overlap the time. Right now there's no distance between them:
    image

A possible fix could be setting some extra distance between the time containers. For example even 10 extra pixels visually separate the menu from the next time block:
image

What to discuss:

  • Does increasing distance between time blocks solve this issue?
  • What extra distance is enough?

Vertical alignment

Time areas are not aligned into the columns when the participants have structure levels with different lenghts. The smaller the screen, the more noticeable this problem is.
image

Possible solution:

  • Make the layout less flexible by creating a template grid - this will place the widgets strictly in columns, no matter how wide the separate widgets are

Handling long structure level names

Users are allowed to create long structure level names. If they do so, it makes the widgets positioning less consistent:
image

Possible solution is to limit the widget width and wrap the text if it's too long. In this example the widget width is 2 times bigger than the time width:
image

However, this change would make the widgets taller. Is that okay?

Top line width

Should its length remain dependent on the structure level text width (like on all the first 3 screenshots) or be fixed (like on the last one)? Second option looks neater to me.

@vkrasnovyd vkrasnovyd assigned vkrasnovyd and unassigned MSoeb Sep 18, 2024
@Elblinator Elblinator assigned MSoeb and unassigned vkrasnovyd Sep 18, 2024
MSoeb
MSoeb previously requested changes Sep 24, 2024
Copy link

@MSoeb MSoeb left a comment

Choose a reason for hiding this comment

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

The fix needs some improvements and best a conversation about it.

  1. In the current state the fix leads to to the situation, that the mat cards left to the speaking times widget are smaller. This is not good. Especially on on smaller screen sizes, like tablets, this isn't user friendly. -> The mat card width should not be changed. Changes should only be made inside the cards.

  2. The three-dot-menu is still not optimal places. The elements can't easily be dinstingushed if the strcuture levels have a color. It must ne easier to separate the logical elements groups from each other. -> A solution could be to cut off the color a bit. Emanuel and I think, that a time in the style -999:99 should be the max value. The color should be trimmed around this value.

  3. The three-dot-menu is still no far from the number. Maybe the second CR can help, but I think, we should place is a bit nearer to the number even if this would lead to an overlapping of the elements.

@MSoeb MSoeb assigned vkrasnovyd and unassigned MSoeb Sep 24, 2024
@MSoeb
Copy link

MSoeb commented Sep 26, 2024

Please excuse me for not responding directly to your suggestions. I had only looked at the PR and based my CR on it. That was not correct.

About the implementation: The last picture looks good. The arrangement along a grid is a good idea. However, a few points would need to be considered.

  • The overall width of the widget and the width of the other elements to the left of the widget must be maintained. The widget must not become larger, otherwise it will reduce the size of the other elements.

  • In the widget itself, the color bar above the numbers should be shortened. The length of the bar is based on a maximum value for the speaking time. This is -999:59. The minus is required as an additional value, as the time can become negative. The aim is to separate and recognize the individual logical elements more clearly. - TLDR: Please shorten the color bar so that a number in the format -999:59 is covered.

  • The idea with the line break sounds good. Please implement it as in the last image.

Context for the second key point: The three-dot menu appears detached from the associated element or, due to the length of the color strokes, it currently appears that this menu could also belong to another element. I hope that the above changes will be sufficient to make it easier to distinguish the elements that belong together.

@vkrasnovyd vkrasnovyd closed this Oct 7, 2024
@vkrasnovyd vkrasnovyd deleted the fix-speaking-times-widget branch October 7, 2024 09:02
@vkrasnovyd vkrasnovyd restored the fix-speaking-times-widget branch October 7, 2024 09:03
@vkrasnovyd vkrasnovyd reopened this Oct 7, 2024
@vkrasnovyd
Copy link
Contributor Author

The list of changes:

  • Time widgets are grouped into columns of the same width.
  • Length of color line is now equal to the length of '-999:59' time - 111px.
  • Min column width for phones (screen size < 500px) is 159px: 111px line length + 48px menu-dots area.
  • Min column width for bigger screens is 186px: (111px + 25%) + 48px.
  • Max column width (including menu-dots area) is 222px - twice more than the length of time widget.
  • Limited the width of the right column for the screens where only 1 column can fit - to avoid columns becoming too wide.

Changes for phones (screen size < 500px)

  • Reduced the paddings outside the grid - now they are the same as in the other autopilot components.
  • Reduced distance between menu-dots and right widget border.
  • Set smaller than on bigger screens horyzontal gap between grid columns.

@MSoeb if some proportins still don't feel right, we can arrange a meeting to check different size options together and find the best combination.

@vkrasnovyd vkrasnovyd requested a review from MSoeb October 7, 2024 10:05
@bspekker bspekker merged commit 0c4ec2a into OpenSlides:main Nov 18, 2024
2 checks passed
@vkrasnovyd vkrasnovyd deleted the fix-speaking-times-widget branch November 29, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Speaking Times Widget: Layout error
7 participants