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

Make Applications visible by default in the app navigation bar #1250

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Jul 29, 2024

Contributes to #1177

In 0.7.* we are displaying Tables Applications by default and without further exposed options by default in the navigation bar. The intended default was hidden, but keeping it would break the experience, and may make them less discoverable in general.

@blizzz blizzz added enhancement New feature or request 3. to review Waiting for reviews labels Jul 29, 2024
$permissionUpdate,
$permissionDelete,
$permissionManage,
Application::NAV_ENTRY_MODE_ALL
Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW it is not really applicable here (table share…), but have the same default may lead to less confusion in the future us's.

@blizzz
Copy link
Member Author

blizzz commented Jul 29, 2024

Mh, OpenAPI (not committed yet) has an issue with the constant. Neither does the command deduct the default value, but it also considers it required, instead of optional:

diff --git a/openapi.json b/openapi.json
index 652d6d5c..ea5b1c45 100644
--- a/openapi.json
+++ b/openapi.json
@@ -2439,7 +2439,8 @@
                                     "nodeId",
                                     "nodeType",
                                     "receiver",
-                                    "receiverType"
+                                    "receiverType",
+                                    "displayMode"
                                 ],
                                 "properties": {
                                     "nodeId": {
@@ -2496,7 +2497,6 @@
                                     "displayMode": {
                                         "type": "integer",
                                         "format": "int64",
-                                        "default": 0,
                                         "description": "context shares only, whether it should appear in nav bar. 0: no, 1: recipients, 2: all"
                                     }
                                 }

@blizzz blizzz force-pushed the enh/1177/application-default-show branch from 9109ba2 to 5681875 Compare July 29, 2024 18:40
@blizzz
Copy link
Member Author

blizzz commented Jul 29, 2024

Mh, OpenAPI (not committed yet) has an issue with the constant. Neither does the command deduct the default value, but it also considers it required, instead of optional:

diff --git a/openapi.json b/openapi.json
index 652d6d5c..ea5b1c45 100644
--- a/openapi.json
+++ b/openapi.json
@@ -2439,7 +2439,8 @@
                                     "nodeId",
                                     "nodeType",
                                     "receiver",
-                                    "receiverType"
+                                    "receiverType",
+                                    "displayMode"
                                 ],
                                 "properties": {
                                     "nodeId": {
@@ -2496,7 +2497,6 @@
                                     "displayMode": {
                                         "type": "integer",
                                         "format": "int64",
-                                        "default": 0,
                                         "description": "context shares only, whether it should appear in nav bar. 0: no, 1: recipients, 2: all"
                                     }
                                 }

I dug down that hole. In the PHP parser used, we can get the class and the name of the constant, but not its value. Reflection is not an option there. So I changed it back to literal value in the Api1Controller.

UPDATE: @provokateurin confirmed in chat this is the way to go for now.

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 enhancement New feature or request
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

1 participant