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

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

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

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Jul 12, 2024

contributes to #1177

It was temporarily disabled and shown by default as we did not have web UI controls to configure it.

@blizzz blizzz added enhancement New feature or request 2. developing Work in progress labels Jul 12, 2024
@blizzz blizzz self-assigned this Jul 17, 2024
@blizzz blizzz force-pushed the enh/1177/enable-nav-display-logic branch from 2a6a88f to 52638ee Compare July 22, 2024 22:10
@blizzz blizzz marked this pull request as ready for review July 22, 2024 22:11
@blizzz blizzz added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 22, 2024
@blizzz blizzz requested review from juliusknorr, elzody and enjeck July 22, 2024 22:11
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Backend change looks good 👍

@enjeck enjeck force-pushed the enh/1177/enable-nav-display-logic branch from 52638ee to ac42f69 Compare August 6, 2024 18:45
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Let me block the merge just to be sure this doesn't get lost from the initial post

⚠️ merge only with the frontend tasks as laid out in #1177 (comment)

@enjeck
Copy link
Contributor

enjeck commented Aug 6, 2024

Let me block the merge just to be sure this doesn't get lost from the initial post

⚠️ merge only with the frontend tasks as laid out in #1177 (comment)

Yeah, I'm looking into the frontend parts :D

@juliusknorr
Copy link
Member

Great, thanks :)

@enjeck
Copy link
Contributor

enjeck commented Aug 22, 2024

It seems there's no way for an owner to modify their navigation entry if there are no shares? Creating an application without shares means there's no navigation entry, and no way to change it. @blizzz

@blizzz
Copy link
Member Author

blizzz commented Sep 2, 2024

It seems there's no way for an owner to modify their navigation entry if there are no shares? Creating an application without shares means there's no navigation entry, and no way to change it. @blizzz

This is accurate. We bound the nav bar display state to sharing. Also the share id is part of the primary key.

I do see it is reasonable to have this setting for non-shared applications, but spontaneously a quick solution that is not too terrible does not come to my mind. Is it a blocker?

@enjeck
Copy link
Contributor

enjeck commented Sep 2, 2024

It seems there's no way for an owner to modify their navigation entry if there are no shares? Creating an application without shares means there's no navigation entry, and no way to change it. @blizzz

This is accurate. We bound the nav bar display state to sharing. Also the share id is part of the primary key.

I do see it is reasonable to have this setting for non-shared applications, but spontaneously a quick solution that is not too terrible does not come to my mind. Is it a blocker?

Maybe not a blocker, but still inconvenient and not very user-friendly. The UI right now doesn't look great, and I can't seem to fix it due to this: #1295

@blizzz
Copy link
Member Author

blizzz commented Sep 2, 2024

It seems there's no way for an owner to modify their navigation entry if there are no shares? Creating an application without shares means there's no navigation entry, and no way to change it. @blizzz

This is accurate. We bound the nav bar display state to sharing. Also the share id is part of the primary key.
I do see it is reasonable to have this setting for non-shared applications, but spontaneously a quick solution that is not too terrible does not come to my mind. Is it a blocker?

Maybe not a blocker, but still inconvenient and not very user-friendly. The UI right now doesn't look great, and I can't seem to fix it due to this: #1295

Yup, I am with you there. Need to think about a good strategy.

@enjeck enjeck force-pushed the enh/1177/enable-nav-display-logic branch from ac42f69 to ccf873b Compare September 4, 2024 08:26
@enjeck enjeck force-pushed the enh/1177/enable-nav-display-logic branch from ccf873b to 8075a22 Compare September 19, 2024 09:51
@enjeck 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 enjeck force-pushed the enh/1177/enable-nav-display-logic branch from cd00c05 to bc6ae95 Compare November 19, 2024 07:08
@blizzz
Copy link
Member Author

blizzz commented Nov 19, 2024

Screenshot_20241119_212920

@enjeck I feel a bit disconnected. That toggle there, changing it does not have any effect via GUI.

It was temporarily disabled and shown by default as we did not have web UI
controls to configure it.

Signed-off-by: Arthur Schiwon <[email protected]>
@enjeck enjeck force-pushed the enh/1177/enable-nav-display-logic branch from 99b8922 to 40a2485 Compare November 27, 2024 09:14
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.

3 participants