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

Improve sidebar organization #6129

Merged
merged 1 commit into from
Jul 15, 2024
Merged

Improve sidebar organization #6129

merged 1 commit into from
Jul 15, 2024

Conversation

GVodyanov
Copy link
Contributor

Close #5083

A B
image image

@GVodyanov GVodyanov added the 3. to review Waiting for reviews label Jul 8, 2024
@GVodyanov GVodyanov self-assigned this Jul 8, 2024
Copy link

codecov bot commented Jul 8, 2024

Codecov Report

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

Project coverage is 23.99%. Comparing base (9802787) to head (6bebd61).
Report is 3 commits behind head on main.

Files Patch % Lines
src/components/AppNavigation/CalendarList.vue 0.00% 22 Missing ⚠️
...ts/AppNavigation/CalendarList/CalendarListItem.vue 0.00% 9 Missing ⚠️
...nts/AppNavigation/CalendarList/CalendarListNew.vue 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6129      +/-   ##
============================================
- Coverage     29.84%   23.99%   -5.85%     
+ Complexity      914      457     -457     
============================================
  Files           289      247      -42     
  Lines         13768    11576    -2192     
  Branches       2165     2186      +21     
============================================
- Hits           4109     2778    -1331     
+ Misses         9344     8483     -861     
  Partials        315      315              
Flag Coverage Δ
javascript 15.49% <0.00%> (-0.08%) ⬇️
php 59.52% <ø> (ø)

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.

Copy link
Member

@nimishavijay nimishavijay left a comment

Choose a reason for hiding this comment

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

Awesome! Looks super nice :)

Only one minor point of feedback: since there is now a heading for the deck section, would it be possible to remove the repeated "Deck: " in the beginning of those calendars?

Can't tell from the screenshot, but if a section doesn't have any calendars it should be hidden.

Approving to not block, they can also be follow ups 🚀

@Ornanovitch
Copy link

If I may, I love this improvement but the "hidden" section seems too much: a disabled calendar might be a Deck one, a Shared one... Disabled is more a state than a category, IMO a disabled calendar should be grayed out in its proper section. :)

@GVodyanov
Copy link
Contributor Author

Hey everyone!

Screenshot from 2024-07-12 12-22-39

Here is what it looks like now: no custom CSS, sections are hidden if empty, "Deck: " removed, and the hidden menu is implemented with allowCollapse of NcAppNavigationItem.

@nimishavijay @ChristophWurst

@ChristophWurst
Copy link
Member

The headings and the checkboxes are marginally misaligned. Perhaps this could be addressed in nc/vue? I assume other apps using the same components could have similar alignment issues

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Please squash the three commits into one for a clean history and atomic changes :)

Signed-off-by: Grigory Vodyanov <scratchx@gmx.com>
@ChristophWurst ChristophWurst merged commit bf56092 into main Jul 15, 2024
40 of 41 checks passed
@ChristophWurst ChristophWurst deleted the enh/sidebar-organization branch July 15, 2024 08:05
@miaulalala miaulalala added this to the v5.0.0 milestone Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve sidebar display of Calendars
5 participants