npu icon indicating copy to clipboard operation
npu copied to clipboard

Don't forcefully hide header with CSS

Open Balint66 opened this issue 3 years ago • 7 comments

There is a "button" under the header, to hide it. Maybe NPU could make it more visible so nobody could miss it.

It should solve #67

Balint66 avatar Jan 10 '22 22:01 Balint66

Hey @FeaXR thanks for your work, but I don't think #75 fixes this issue, all that's changed for me is that the button for showing/hiding the header is visible, but the header will be hidden again on page refresh even if I enable it.

What I don't get is why NPU would even touch the header. Maybe in the past there was no built-in way to hide it (?), but now there is, so I see no use for this feature. I think you should just remove all code related to hiding the header.

I have never used node, but I could do a PR if somebody from the project approves this change.

boapps avatar Jan 13 '22 14:01 boapps

@boapps In the past when this feature was introduced there was no way to hide the header. Apparently the developers of Neptun have finally realized after ~10 years that this was a sensible feature to have and added it themselves.

I have a few concerns about @FeaXR's PR myself which I will address in my review soon.

solymosi avatar Jan 13 '22 14:01 solymosi

@boapps The point of hiding the header is to free up half the screen, that is basically wasted by a non-fuctional element By changing how the hiding works I added the possibility to toggle in the header manually if the user sees fit

FeaXR avatar Jan 13 '22 14:01 FeaXR

@solymosi

In the past when this feature was introduced there was no way to hide the header

makes sense, thanks for the info

@FeaXR

By changing how the hiding works I added the possibility to toggle in the header manually

Neptun can do that already and that actually works, while I've found no way to enable the header persistently with your PR

boapps avatar Jan 13 '22 14:01 boapps

We should just remove the hiding functionality altogether. What we could leave behind is code that hides it a single time but then leaves things alone, so if the user wants to unhide it, they can. This would ensure continuity in behavior from previous versions of NPU for existing users.

solymosi avatar Jan 13 '22 14:01 solymosi

@solymosi I'll close my PR #75 then, and when I have time I'll get the new mode of hiding only once working, then I'll open a new PR. The changes in #75 that are in #74 can be still included if you se fit, so only the changes to the hiding method can be disregarded

@boapps Thanks for the correction, I didn't now about the closing button at all, but this new way of hiding only once seems like a good solution to me.

FeaXR avatar Jan 13 '22 15:01 FeaXR

I think solymosi's solution would be the best way to do it. Neptun uses a ShowHeader(buttonObject) on the button, that actually hides the PageMethods.SaveHeaderState(visibility). In my opinion, on first login, we could use either of those functions to hide the header, then let Neptun and the user handle the things.

Another thing, that the sidebar has the same functionality, and when it's hidden, on pages that are actually pretty small (like Grade average) the website becomes very compact. Maybe on this the same thing could be applied like on the header.

Balint66 avatar Jan 22 '22 08:01 Balint66

Closing as this will be addressed in the next release of NPU.

solymosi avatar Dec 09 '22 10:12 solymosi