Unify navbars #233
Unify navbars #233
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #233 +/- ##
==========================================
- Coverage 26.39% 26.13% -0.27%
==========================================
Files 116 116
Lines 5076 5127 +51
Branches 101 101
==========================================
Hits 1340 1340
- Misses 3651 3702 +51
Partials 85 85 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…main in top right nav menu for logged out users
… menu to top right using VerticalBars(Hamburger) Icon, swipe in from right * currently overlaps with the (empty) mobile menu, so will move the stuff from there into this one menu button for mobile when logged in. * change default page load to be the Kinder (Kids overview) page
…ister) initially.
…n homepage. Change Login or Register translation to "Einloggen" only.
…utton consistent with the menu one
…re it needs to appear * remove the Impressum/Public links from the menu for logged in users when on userLand
…that the "public menu" disappears once in user land, when you navigate to it clientside rather than reloading/loading the page. * remove unused imports
…le, and horizontally with a good margin on desktop
for more information, see https://pre-commit.ci
|
This version has: In Userland view, all options are now in the Sidebar, for both mobile and PC users. (So Language selection and Dark mode(reintroduced) are here). The public buttons (Downloads, Contact, and Log in) only appear on non-user land pages, like the home page. The dark mode and language selection are with them on those pages. Because of the above, on mobile there is now only ever one button - in the top right Users can open the Sidebar from the right with it when logged in on userLand, and it displays the public buttons from the PC in the menu when on other pages, like "Downloads", or Language Selection. |
|
@jmsssc is this ready for review ? |
Yep, this is ready |
There was a problem hiding this comment.
Thanks, this looks good!
A couple of issues I noticed:
- when logged in, trying to change the locale doesn't change the locale, and appears to also reload the "kinder" page in de locale?
- when logged in with a mobile size screen, sometimes clicking on the locale chooser opens the menu at the top of the screen instead of where I clicked
- could this be because in LocaleChooser the Dropdown has
triggeredBy="#id", with the same id for both of our instances of LocaleChooser?
- could this be because in LocaleChooser the Dropdown has
- "einstellung" link doesn't seem to work
- (very minor) the hamburger in mobile view & dark mode is a bit bigger than the top grey area - and maybe would benefit from being white-ish in dark mode to make it more obvious? I guess ideally it would be the same icon that is there in mobile view when the user is not logged in?
| import { MoonSolid, SunSolid } from "flowbite-svelte-icons"; | ||
| </script> | ||
|
|
||
| <FunctionalIcon tooltip={'Darkmode ein- oder ausschalten'}> |
There was a problem hiding this comment.
this german tooltip text can be added somewhere in https://github.com/ssciwr/mondey/blob/main/frontend/src/lib/translations.ts
and referenced here, e.g. something like
...
import { i18n } from "$lib/i18n.svelte";
...
tooltip={i18n.tr.frontpage.toggleDarkmode}then in the admin interface in the deployed version translations can be added for other languages
There was a problem hiding this comment.
Thank you for the review @lkeegan
Ah good catch sure absolutely the Hamburger icon is an issue on dark mode, that's wrong. I made the icon bigger but I can return it to the same size as the non-logged in icon (lg rather than xl) and make it turn white in dark mode. And maybe remove the background color of it's container if it messes with the UI. I will make the tooltip a translation key too.
On the other couple of things: The LocaleChooser I thought wasn't functional yet throughout, but absolutely I will look at why it might not work now and get it working on this branch. Einstellung is the same, it was like that so I did not change it or think anything was awry as I didn't realize it worked differently on main. I will look at code in main where I see it allows you to change your password and set locale, and get it working on this branch.
|
open still for suggestions:
separate PR: