feat: Add UI elements to modify navigation display #1295

enjeck · 2024-08-16T19:22:17Z

Todo

ensure it works when you update a context
ensure it works for owners and recipients too
Discuss the UI and copy
Ensure self-shares to the owner are deleted when share is deleted

lib/Service/ShareService.php

src/modules/navigation/partials/NavigationContextItem.vue

enjeck · 2024-12-16T06:45:14Z

Okay. Took a long time, but this finally works (from my tests). If others confirm, we should be good to merge it (after adding tests).

UI looks like this: @jancborchardt

blizzz · 2024-12-16T20:40:22Z

One little thing needed: with switching away from the three modes to just two modes, we also have to remove this condition:

tables/lib/Db/ContextMapper.php

Lines 189 to 190 in 4b7e5a7

    
           // an owner (no explicit check necessary): requires ALL 
        
           $qb->expr()->eq('n.display_mode', $qb->createNamedParameter(Application::NAV_ENTRY_MODE_ALL, IQueryBuilder::PARAM_INT)),

Otherwise a share owner cannot remove the entry for themselves. Works fine then!

In the backend, we should remove the third option, now that is is unnecessary.

src/shared/constants.js

Signed-off-by: Cleopatra Enjeck M <[email protected]>

Signed-off-by: Arthur Schiwon <[email protected]>

Signed-off-by: Cleopatra Enjeck M <[email protected]>

Signed-off-by: Arthur Schiwon <[email protected]>

… column Signed-off-by: Arthur Schiwon <[email protected]>

Signed-off-by: Cleopatra Enjeck M. <[email protected]>

- removed to only fetch the one share that matches the user id. Group shares my be present and only those relevant to the current user are being fetched. - setting the override now against any share. There might be more and some might be dropped – the override is working against any of them - one none-hidden setting wins - dropped using the NAV_ENTRY_MODE_RECIPIENTS as it is not needed anymore Signed-off-by: Arthur Schiwon <[email protected]>

Signed-off-by: Arthur Schiwon <[email protected]>

Signed-off-by: Cleopatra Enjeck M. <[email protected]>

lib/Controller/NavigationController.php

this can be removed once >=NC31 is supported and our NavigationController is adjusted Signed-off-by: Arthur Schiwon <[email protected]>

juliusknorr · 2025-01-07T10:38:06Z

src/modules/modals/CreateContext.vue

@@ -42,6 +42,14 @@
 				</div>
 				<NcContextResource :resources.sync="resources" :receivers.sync="receivers" />
 			</div>
+			<div class="row space-T">
+				<NcActionCheckbox :checked="showInNavigationDefault" @change="updateDisplayMode">
+					Show in app list


Should be translatable

juliusknorr · 2025-01-07T10:38:10Z

src/modules/modals/CreateContext.vue

+					Show in app list
+				</NcActionCheckbox>
+				<p class="nav-display-subtext">
+					This can be overridden by a per-account preference


Should be translatable

juliusknorr · 2025-01-07T10:38:49Z

src/modules/modals/EditContext.vue

-
+			<div class="row space-T">
+				<NcActionCheckbox :checked="showInNavigationDefault" @change="updateDisplayMode">
+					Show in app list


Should be translatable

juliusknorr · 2025-01-07T10:38:53Z

src/modules/modals/EditContext.vue

+					Show in app list
+				</NcActionCheckbox>
+				<p class="nav-display-subtext">
+					This can be overridden by a per-account preference


Should be translatable

juliusknorr · 2025-01-07T10:39:05Z

src/modules/navigation/partials/NavigationContextItem.vue

 				<template #icon>
 					<Delete :size="20" />
 				</template>
 				{{ t('tables', 'Delete application') }}
 			</NcActionButton>
+			<NcActionCheckbox :checked="showInNavigation" @change="updateDisplayMode">
+				Show in app list


Should be translatable

juliusknorr

Small nitpicks, but otherwise looks good 👍

Signed-off-by: Julius Knorr <[email protected]>

juliusknorr

Tested and works. Also reviewed the backend bits of the base PR.

enjeck self-assigned this Aug 16, 2024

enjeck commented Aug 22, 2024

View reviewed changes

lib/Service/ShareService.php Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

juliusknorr added enhancement New feature or request 2. developing Work in progress labels Aug 29, 2024

enjeck mentioned this pull request Sep 2, 2024

feat(Contexts): enable nav bar display logic #1193

Merged

2 tasks

enjeck force-pushed the enh/1177/enable-nav-display-logic branch from ac42f69 to ccf873b Compare September 4, 2024 08:26

enjeck force-pushed the nav-display-frontend branch from e0b033f to 215ca74 Compare September 4, 2024 08:29

enjeck force-pushed the enh/1177/enable-nav-display-logic branch from ccf873b to 8075a22 Compare September 19, 2024 09:51

enjeck force-pushed the nav-display-frontend branch from 52553e6 to 37e8272 Compare September 19, 2024 09:59

This comment was marked as outdated.

Sign in to view

enjeck marked this pull request as ready for review October 3, 2024 08:21

enjeck commented Oct 3, 2024

View reviewed changes

src/modules/navigation/partials/NavigationContextItem.vue Outdated Show resolved Hide resolved

src/modules/navigation/partials/NavigationContextItem.vue Outdated Show resolved Hide resolved

enjeck force-pushed the enh/1177/enable-nav-display-logic branch 2 times, most recently from 0edd48a to cd00c05 Compare October 30, 2024 07:33

enjeck force-pushed the enh/1177/enable-nav-display-logic branch from cd00c05 to bc6ae95 Compare November 19, 2024 07:08

enjeck requested a review from blizzz as a code owner November 19, 2024 07:08

enjeck force-pushed the nav-display-frontend branch from 4986b0f to 0e62738 Compare November 20, 2024 05:41

enjeck force-pushed the enh/1177/enable-nav-display-logic branch from 99b8922 to 40a2485 Compare November 27, 2024 09:14

enjeck force-pushed the nav-display-frontend branch from 2f01848 to 93126f1 Compare December 13, 2024 16:29

enjeck force-pushed the enh/1177/enable-nav-display-logic branch from 40a2485 to c144b4e Compare December 16, 2024 06:25

enjeck force-pushed the nav-display-frontend branch 2 times, most recently from e863887 to ab66fcc Compare December 16, 2024 06:38

enjeck requested review from jancborchardt and juliusknorr December 16, 2024 06:45

blizzz reviewed Dec 16, 2024

View reviewed changes

src/shared/constants.js Outdated Show resolved Hide resolved

enjeck and others added 15 commits January 4, 2025 15:16

feat: add UI elements to change nav display

7f8da4a

Signed-off-by: Cleopatra Enjeck M <[email protected]>

fix: update context navigation display elements

afcbe45

Signed-off-by: Cleopatra Enjeck M <[email protected]>

fix(Context): fix wrong parameters to permission check

e05745a

Signed-off-by: Arthur Schiwon <[email protected]>

fix(DB): override mapper::update() where tables has no id column

6a693aa

Signed-off-by: Arthur Schiwon <[email protected]>

fix: use NcDialog

04cd53e

Signed-off-by: Cleopatra Enjeck M <[email protected]>

fix: properly update navigation display via checkbox

41b5b24

Signed-off-by: Cleopatra Enjeck M <[email protected]>

enh: add share to oneself to control context nav display

4be1a7c

Signed-off-by: Cleopatra Enjeck M <[email protected]>

enh: use checkbox for default nav display mode

bd37daf

Signed-off-by: Cleopatra Enjeck M <[email protected]>

enh: modify display mode update for owner

1ab79e7

Signed-off-by: Cleopatra Enjeck M <[email protected]>

fix(Navigation): use default display mode when there is no user override

b00f1c0

Signed-off-by: Arthur Schiwon <[email protected]>

fix(DB): NavigationMapper has to override insert due to not having an id

454844e

… column Signed-off-by: Arthur Schiwon <[email protected]>

fix: update display mode when editing context

efab19c

Signed-off-by: Cleopatra Enjeck M. <[email protected]>

feat(Contexts): dynamic navigation bar update on config toggle

4ab44af

Signed-off-by: Arthur Schiwon <[email protected]>

enh: remove usages of NAV_ENTRY_MODE_RECIPIENTS

e1e3737

Signed-off-by: Cleopatra Enjeck M. <[email protected]>

enjeck force-pushed the nav-display-frontend branch from e0a2cd8 to 1e0c152 Compare January 4, 2025 19:26

fix: add copyright and update openapi

30748c4

Signed-off-by: Cleopatra Enjeck M. <[email protected]>

enjeck force-pushed the nav-display-frontend branch from 1e0c152 to 30748c4 Compare January 4, 2025 19:59

enjeck commented Jan 4, 2025

View reviewed changes

lib/Controller/NavigationController.php Show resolved Hide resolved

ci: suppress psalm warning against private NavigationController class

c925f42

this can be removed once >=NC31 is supported and our NavigationController is adjusted Signed-off-by: Arthur Schiwon <[email protected]>

juliusknorr reviewed Jan 7, 2025

View reviewed changes

fix: Make labels translatable

4d5a063

Signed-off-by: Julius Knorr <[email protected]>

juliusknorr approved these changes Jan 9, 2025

View reviewed changes

blizzz merged commit 76b51d8 into enh/1177/enable-nav-display-logic Jan 9, 2025
58 checks passed

blizzz deleted the nav-display-frontend branch January 9, 2025 21:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add UI elements to modify navigation display #1295

feat: Add UI elements to modify navigation display #1295

enjeck commented Aug 16, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

enjeck commented Dec 16, 2024 •

edited

Loading

blizzz commented Dec 16, 2024

juliusknorr Jan 7, 2025

juliusknorr Jan 7, 2025

juliusknorr Jan 7, 2025

juliusknorr Jan 7, 2025

juliusknorr Jan 7, 2025

juliusknorr left a comment

juliusknorr left a comment

feat: Add UI elements to modify navigation display #1295

feat: Add UI elements to modify navigation display #1295

Conversation

enjeck commented Aug 16, 2024 • edited Loading

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

enjeck commented Dec 16, 2024 • edited Loading

blizzz commented Dec 16, 2024

juliusknorr Jan 7, 2025

Choose a reason for hiding this comment

juliusknorr Jan 7, 2025

Choose a reason for hiding this comment

juliusknorr Jan 7, 2025

Choose a reason for hiding this comment

juliusknorr Jan 7, 2025

Choose a reason for hiding this comment

juliusknorr Jan 7, 2025

Choose a reason for hiding this comment

juliusknorr left a comment

Choose a reason for hiding this comment

juliusknorr left a comment

Choose a reason for hiding this comment

enjeck commented Aug 16, 2024 •

edited

Loading

enjeck commented Dec 16, 2024 •

edited

Loading