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: 13818 create StudioContentMenu #13847

Closed
wants to merge 4 commits into from

Conversation

standeren
Copy link
Contributor

@standeren standeren commented Oct 23, 2024

Description

Create StudioContentMenu as a reusable component across SettingsModal, ResourceAdm and soon in ContentLibrary.

Settingsmodal

Old menu in settingsmodal:
Skjermbilde 2024-10-23 kl  15 24 58

New menu in settingsmodal:
Skjermbilde 2024-10-23 kl  15 20 35

ResourceAdm

Old menu in ResourceAdm:
Skjermbilde 2024-10-23 kl  15 27 36

New menu in ResourceAdm:
Skjermbilde 2024-10-23 kl  15 23 43

Related Issue(s)

  • #{issue number}

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)

@standeren standeren linked an issue Oct 23, 2024 that may be closed by this pull request
@github-actions github-actions bot added solution/studio/designer Issues related to the Altinn Studio Designer solution. frontend labels Oct 23, 2024
@standeren standeren force-pushed the 13818-create-studiocontentmenu branch 2 times, most recently from 68e7dcd to 7d95a94 Compare October 23, 2024 13:26
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 94.79167% with 5 lines in your changes missing coverage. Please review.

Project coverage is 95.28%. Comparing base (4333644) to head (42ad227).
Report is 31 commits behind head on main.

Files with missing lines Patch % Lines
...ntentMenu/StudioMenuTab/StudioMenuTabContainer.tsx 80.00% 3 Missing ⚠️
...rc/components/StudioContentMenu/utils/dom-utils.ts 91.30% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13847      +/-   ##
==========================================
- Coverage   95.29%   95.28%   -0.01%     
==========================================
  Files        1653     1653              
  Lines       22024    22022       -2     
  Branches     2589     2586       -3     
==========================================
- Hits        20988    20984       -4     
- Misses        790      793       +3     
+ Partials      246      245       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wrt95
Copy link
Contributor

wrt95 commented Oct 24, 2024

Great job implementing this new @studio/component @standeren! 👏
I can see that we are at least doing four different things in this PR:

  • Creating the new component (core)
  • Making use of the new component in settings-modal (domain2)
  • Making use of the new component in resourceadm.
  • Deleting the old component.

As our focus these days are smaller PRs and splitting the them, I suggest splitting this PR into 4 PRs based on the list I wrote above 😄 It will then be easier to review the code, and test the different areas separately. And other reason is so the relevant sub-teams (core, domain2, and resourceadm) will be able to review and test the code in «their area» without affecting each other 😄
What do you think about this?

@standeren standeren force-pushed the 13818-create-studiocontentmenu branch from 7d95a94 to 32a03c1 Compare October 24, 2024 06:53
@standeren
Copy link
Contributor Author

Great job implementing this new @studio/component @standeren! 👏 I can see that we are at least doing four different things in this PR:

  • Creating the new component (core)
  • Making use of the new component in settings-modal (domain2)
  • Making use of the new component in resourceadm.
  • Deleting the old component.

As our focus these days are smaller PRs and splitting the them, I suggest splitting this PR into 4 PRs based on the list I wrote above 😄 It will then be easier to review the code, and test the different areas separately. And other reason is so the relevant sub-teams (core, domain2, and resourceadm) will be able to review and test the code in «their area» without affecting each other 😄 What do you think about this?

Can try to do so!

@standeren standeren force-pushed the 13818-create-studiocontentmenu branch from 32a03c1 to 42ad227 Compare October 24, 2024 06:59
@standeren
Copy link
Contributor Author

Here is the first: #13898
Need to wait for this one to be merged before I can create the others

@wrt95
Copy link
Contributor

wrt95 commented Oct 24, 2024

Here is the first: #13898 Need to wait for this one to be merged before I can create the others

Great @standeren 😄 I'll take a look at it asap 🚀
You can still create the others as draft I believe?

@standeren
Copy link
Contributor Author

You can still create the others as draft I believe?

Yes, but wont those need to include the changes in the first one anyway? Of course I can merge with main when the contentMenu is merged. I'll see what I find best to do!

@standeren standeren marked this pull request as draft October 24, 2024 10:28
@standeren
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend solution/studio/designer Issues related to the Altinn Studio Designer solution. team/studio-core team/studio-domain1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants