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 calendar lists not being orderable any more #6536

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

GVodyanov
Copy link
Contributor

Fix #6501

So the problem with this bug is that the way that dragging the calendars previously worked is conceptually incompatible with the division of calendars into different sections, mostly because of the hidden calendars. The hidden calendars being part of the order, but being in a different place from all the others, causes the order coming from vue-draggable to be wrong. Another issue discussed with Christoph is the drag to reorder feature is pretty hard to discover.

Here are a few things I thought about:

  • Instead of having one big orderable section, have 3, one for personal, one for shared, and one for deck. This however still doesn't fix the issue of hidden calendars.
  • One potential solution to the hidden calendars is making all calendars unhidden when the users starts dragging, but this would be pretty jarring.
  • Have some sort of button indicating that you are able to reorder the calendars (should probably talk with designers)

In the end to make this happen I thought of adding a button which opens a modal with the three sections: personal, shared, and deck, and WITHOUT the hidden calendars section.

@GVodyanov GVodyanov added the 2. developing Work in progress label Nov 30, 2024
@GVodyanov GVodyanov self-assigned this Nov 30, 2024
Copy link

codecov bot commented Nov 30, 2024

Codecov Report

Attention: Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.

Project coverage is 23.30%. Comparing base (f77d285) to head (619f4f0).
Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
src/components/AppNavigation/CalendarList.vue 0.00% 17 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6536      +/-   ##
============================================
- Coverage     23.33%   23.30%   -0.03%     
  Complexity      453      453              
============================================
  Files           248      248              
  Lines         11777    11791      +14     
  Branches       2245     2257      +12     
============================================
  Hits           2748     2748              
- Misses         8705     8719      +14     
  Partials        324      324              
Flag Coverage Δ
javascript 14.92% <0.00%> (-0.03%) ⬇️
php 59.29% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@GVodyanov
Copy link
Contributor Author

What it could look like:

image

@ChristophWurst
Copy link
Member

I'd say ignore hidden calendars for ordering. Just sort them alphabetically all the time.

For own vs shared calendars vs deck I'd find it OK to split the drag area into three, because you can't drag from owned calendars to shared etc.

Have some sort of button indicating that you are able to reorder the calendars (should probably talk with designers)

That's a bonus, but I'd support that for feature discoverability 👍

@GVodyanov
Copy link
Contributor Author

@ChristophWurst The issue with ignoring hidden calendars is that even though they are hidden, they still make up the "order" of the calendars. So if you have:

  • Calendar ID 1
  • Calendar ID 2 (hidden)
  • Calendar ID 3
  • Calendar ID 4

With the order [1, 2, 3, 4]

The draggable list would show:

  • Calendar ID 1
  • Calendar ID 3
  • Calendar ID 4

Now imagine you move Calendar ID 4 to the second position, resulting in:

  • Calendar ID 1
  • Calendar ID 4
  • Calendar ID 3

The order could either be [1, 2, 4, 3] or [1, 4, 2, 3] seeing as the hidden calendar, even if not displayed in the UI, still is part of the list. This causes draggable to give weird results.

There are a few ways that come to mind to solve this: show the hidden calendars when dragging (jarring because they appear out of nowhere), append hidden calendars to the end (messing up the order when u want to hide a calendar), etc. The modal is also one.

@ChristophWurst
Copy link
Member

append hidden calendars to the end (messing up the order when u want to hide a calendar)

This :)

See hidden list as extension of the visible calendars list. In your example, the order will be [ 1 4 3 2 ]. When you unhide the calendar it will be at the bottom of the visible calendars, which seems acceptable to me.

@GVodyanov GVodyanov force-pushed the fix/calendar-list-ordering branch from 06552b7 to 619f4f0 Compare December 5, 2024 17:42
@GVodyanov GVodyanov marked this pull request as ready for review December 5, 2024 17:42
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski left a comment

Choose a reason for hiding this comment

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

Tested. Works.

Arranging calendar list order works, unhiding a calendar adds the calendar to the end of the list.

@GVodyanov GVodyanov merged commit 00e21d7 into main Dec 9, 2024
36 of 38 checks passed
@GVodyanov GVodyanov deleted the fix/calendar-list-ordering branch December 9, 2024 18:56
@ChristophWurst
Copy link
Member

/backport to stable5.0

@backportbot backportbot bot added the backport-request A backport was requested for this pull request label Dec 9, 2024
@backportbot backportbot bot removed the backport-request A backport was requested for this pull request label Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drag to reorder calendars in the browser is broken
3 participants