-
Notifications
You must be signed in to change notification settings - Fork 76
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: add org-library and apps menu items and additional routing path #14539
feat: add org-library and apps menu items and additional routing path #14539
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## org-library-mvp #14539 +/- ##
================================================
Coverage 95.72% 95.73%
================================================
Files 1915 1918 +3
Lines 24892 24936 +44
Branches 2840 2837 -3
================================================
+ Hits 23829 23872 +43
Misses 802 802
- Partials 261 262 +1 ☔ View full report in Codecov by Sentry. |
f19a762
to
1d31524
Compare
1d31524
to
765b2d1
Compare
765b2d1
to
db33af3
Compare
ada387a
to
169dbe0
Compare
169dbe0
to
cb08da6
Compare
cb08da6
to
1430db3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fint at du fikk løst opp i rutingen! Jeg ser at vi har behov for å standardisere hvordan vi definerer ruter (nå er det mye frem og tilbake med hardkoding av skråstreker rundt omkring), men det får bli en annen sak.
frontend/dashboard/pages/PageLayout/DashboardHeader/DashboardHeader.tsx
Outdated
Show resolved
Hide resolved
frontend/dashboard/pages/PageLayout/DashboardHeader/dashboardHeaderMenuUtils.ts
Outdated
Show resolved
Hide resolved
frontend/dashboard/pages/PageLayout/DashboardHeader/dashboardHeaderMenuUtils.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ser bra ut!
Det eneste jeg kan sette fingeren på, er at menyelementene er mindre enn i app-development
, og at valgt side ikke er understreket i menyen. Men styling skulle ikke omfavnes av denne PR-en, så det går bra.
Jeg og @Konrad-Simso har også testet denne lokalt, og finner ingen feil. Så jeg setter skip-manual-testing
på denne.
…github.com/Altinn/altinn-studio into add-sub-routes-and-menu-items-on-dashboard
Co-authored-by: Erling Hauan <[email protected]> Co-authored-by: Tomas Engebretsen <[email protected]>
This reverts commit 45757ae.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Har sett over koden og re-testet lokalt. Ser veldig bra ut!
Description
Add an additional sub-route to be able to rename
dashboard
tohome
in the future. This creates a new abstraction level that will place the content-library for org-level on the same hierarchy as apps.Menu items in header are added for both org-library and apps.
Skjermopptak.2025-01-30.kl.12.59.02.mov
Currently the sub route will not be set by default when rendering the dashboard - not sure where to change this? Probably in the load-balancer among other places? And maybe not something we want to change? Maybe
dashboard
should be changed tohome
with a gui and pressing the menu items will add the desired sub route.Out of scope:
dashboard
tohome
self
as contextRelated Issue(s)
Verification