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

System/General - Set default navigation appearance #3279

Closed
wants to merge 7 commits into from
Closed

System/General - Set default navigation appearance #3279

wants to merge 7 commits into from

Conversation

opnsenseuser
Copy link
Member

@opnsenseuser opnsenseuser commented Mar 2, 2019

My starting point for the following consideration was that whenever I log in to the web interface, I get the old menu view even though I almost exclusively use the collapsed menu view.

so i made a new entry in the system - general tab!
You can now select which menu will be displayed automatically after the login.
Saving the selected entry works fine!

grafik

Unfortunately I do not know how to load either the default navigation or the sidebar navigation as standard menu after the login. Can anybody help me further?

Regards
René

opnsenseuser and others added 4 commits March 2, 2019 22:16
My starting point for the following consideration was that whenever I log in to the web interface, I get the old menu view even though I almost exclusively use the collapsed menu view.

so i made a new entry in the system - general tab!
You can now select which menu will be displayed automatically after the login.

Unfortunately I do not know how to load either the default navigation or the sidebar navigation when loading the page. Can anybody help me further?

regards
rené
@opnsenseuser opnsenseuser changed the title System/General - Navigation - choose standard menu System/General - Navigation - set default navigation appearance Mar 2, 2019
@opnsenseuser opnsenseuser changed the title System/General - Navigation - set default navigation appearance System/General - Set default navigation appearance Mar 2, 2019
<td>
<input name="navigation" class="navigation" id="navigation_standard" type="radio" value="standard" <?= $pconfig['navigation'] == "standard" ? 'checked="checked"' : '' ?>/>
<?=gettext("Standard"); ?>
&nbsp;&nbsp;&nbsp;
Copy link
Member

Choose a reason for hiding this comment

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

try to avoid non breaking spaces - it is an ugly hack from old days. you could use inline blocks etc. which have a padding and margin or switch to a select box

<br />
<div class="hidden" data-for="help_for_navigation">
<?=sprintf(
gettext('Set default navigation menu'));?>
Copy link
Member

Choose a reason for hiding this comment

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

remove the sprintf call here - it has no placeholders

@fabianfrz
Copy link
Member

the value is currently never used - you need to integrate them into the frontend - otherwise the setting is never applied.

@opnsenseuser
Copy link
Member Author

@fabianfrz ok. thx for your support. i will do my best and upload it again! 👍 regards rené

@fichtner fichtner self-assigned this Mar 3, 2019
@fichtner
Copy link
Member

fichtner commented Mar 3, 2019

@opnsenseuser this is a good concept, but I'm unsure about the implementation for the moment. let me think about it :)

@opnsenseuser
Copy link
Member Author

@fichtner 👍

opnsenseuser added 2 commits March 3, 2019 10:30
i did an if (!empty) command to set an value if it doesn´t exist.
@opnsenseuser
Copy link
Member Author

opnsenseuser commented Mar 3, 2019

@fabianfrz i changed the things you said and i made an if empty command to set a value if it doesn´t exist. I hope that this command aims to set a default value if nothing is set!

@AdSchellevis
Copy link
Member

@opnsenseuser we probably better save this in your sessionStorage, it's more a user preference than a system preference (same as the expanded advanced section). If you need help storing it in your session storage, I can help you with that.

@opnsenseuser
Copy link
Member Author

@AdSchellevis If you could show me how that works, then I would be very grateful. I'm in a learning phase.

@AdSchellevis
Copy link
Member

@opnsenseuser hmm, while looking at the code, it looks like the intend was already to remember your previous setting:

https://github.com/opnsense/core/blob/master/src/opnsense/www/js/opnsense_theme.js#L200-L202

https://github.com/opnsense/core/blob/master/src/opnsense/www/js/opnsense_theme.js#L209-L212

We probably should have used localstorage in there to outlive the session.
Just to be sure, the intend was to remember my previous dashboard setting after login (on the same device)?

@opnsenseuser
Copy link
Member Author

@AdSchellevis I do not use cookies in my browser. for safety reasons. can it be related to that?

@opnsenseuser
Copy link
Member Author

@AdSchellevis It would still be great if we could set or save this settings to the system. Do you really not want that? then I would delete the request again. Alright? Regards rene

@AdSchellevis
Copy link
Member

@opnsenseuser Personally I don't see a lot of value in it, plus not all theme's might support the feature (leading to questions we don't want to answer). Without cookies enabled, local storage probably won't work. Keep it open for now, we'll discuss internally if/how we want to support this.

@opnsenseuser
Copy link
Member Author

@AdSchellevis 👍

@opnsenseuser
Copy link
Member Author

@AdSchellevis is there any news on this topic? How did you decide?
regards!

@AdSchellevis
Copy link
Member

@opnsenseuser sorry, not yet, other priorities at the moment. We'll leave it open for now.

@opnsenseuser
Copy link
Member Author

@AdSchellevis I'm sorry that I'm talking to you again. Is there any news or could you possibly tell me when you want to talk about the topic? I think this idea is really good, although of course it does not have the highest priority for a firewall. Thanks and best regards, René

@AdSchellevis
Copy link
Member

@opnsenseuser no problem, we just haven't got time to discuss this internally. We'll let you know as soon as we do.

@opnsenseuser
Copy link
Member Author

@AdSchellevis
@fabianfrz
@fichtner
any news on this?
I'm just asking, because a user has contacted me via pm and asked me if this is still in process. I still think the idea is great. what do you think? maybe we can implement that for 20.1? regards rené

@AdSchellevis
Copy link
Member

I don't think we should do this, with cookies enabled in the browser, it already remembers your settings (as stated before)

@opnsenseuser
Copy link
Member Author

i don't use browser cookies for security reasons. furthermore it is technically solved as far as i know, that it only works as long as the current browser session is active. so no cookie is stored on the harddisc.

@fichtner
Copy link
Member

Upon further reflection the current flexibility is nice to have the sidebar enabled on demand. If we make a setting we lose the flexibility to do this because each page load will reset the previous choice. Plus making this functional we need to inject some sort of JavaScript for this config setting to take effect which isn't ideal. For now we need to shelf this until a better strategy is found.

@fichtner fichtner closed this Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants