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: Implement modal to create new code list #14019

Merged
merged 8 commits into from
Nov 22, 2024

Conversation

standeren
Copy link
Contributor

@standeren standeren commented Nov 8, 2024

Description

Skjermopptak.2024-11-08.kl.13.45.59.mov

FYI: Modal has fixed height, same as in ux-editor in code, but not in video ☝️

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)

@github-actions github-actions bot added solution/studio/designer Issues related to the Altinn Studio Designer solution. frontend labels Nov 8, 2024
@standeren standeren force-pushed the 13948-support-for-creating-new-code-list-in-library branch from 85b732a to 91fd085 Compare November 8, 2024 12:51
@standeren standeren changed the title Implement modal to create new option list feat: Implement modal to create new option list Nov 8, 2024
Copy link

codecov bot commented Nov 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.33%. Comparing base (460ceec) to head (feb871d).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14019   +/-   ##
=======================================
  Coverage   95.32%   95.33%           
=======================================
  Files        1780     1782    +2     
  Lines       23159    23202   +43     
  Branches     2689     2690    +1     
=======================================
+ Hits        22077    22120   +43     
  Misses        835      835           
  Partials      247      247           

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


🚨 Try these New Features:

@standeren standeren linked an issue Nov 8, 2024 that may be closed by this pull request
@standeren standeren changed the title feat: Implement modal to create new option list feat: Implement modal to create new code list Nov 8, 2024
@standeren standeren force-pushed the 13948-support-for-creating-new-code-list-in-library branch from 91fd085 to 0207448 Compare November 12, 2024 14:48
@standeren standeren force-pushed the 13948-support-for-creating-new-code-list-in-library branch 8 times, most recently from 0b92066 to e91b8ed Compare November 13, 2024 11:12
@standeren standeren marked this pull request as ready for review November 13, 2024 11:58
@ErlingHauan ErlingHauan self-assigned this Nov 14, 2024
Copy link
Contributor

@ErlingHauan ErlingHauan left a comment

Choose a reason for hiding this comment

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

Nice work! I see that the saving UI pattern here differs from Utforming, where there is no save button, and changes are saved on close. Has this been discussed, or should we consider discussing it?

@ErlingHauan ErlingHauan removed their assignment Nov 14, 2024
@standeren
Copy link
Contributor Author

standeren commented Nov 14, 2024

Nice work! I see that the saving UI pattern here differs from Utforming, where there is no save button, and changes are saved on close. Has this been discussed, or should we consider discussing it?

From what I understood after our meeting last week about saving patterns, I thought this was what we agreed upon 🙈 This is due to the filename has to be provided before the saving can be triggered. So we need a save button to control this interaction. However, when the .altinnstudio file can be taken advantage of, we will not need the save button anymore since we can save the codeList temporarily without a filename in that file 😄

@ErlingHauan
Copy link
Contributor

ErlingHauan commented Nov 14, 2024

Nice work! I see that the saving UI pattern here differs from Utforming, where there is no save button, and changes are saved on close. Has this been discussed, or should we consider discussing it?

From what I understood after our meeting last week about saving patterns, I thought this was what we agreed upon 🙈 This is due to the filename has to be provided before the saving can be triggered. So we need a save button to control this interaction. However, when the .altinnstudio file can be taken advantage of, we will not need the save button anymore since we can save the codeList temporarily without a filename in that file 😄

Sorry, it was not on close in Utforming, but on debounce for the manual editor at least 😅

My impression was that we would first try debounce, and if that could not be implemented, then try onBlur. And if that could not be implemented, have a save button. Just a suggestion, but it might be possible to have a handleSave function that uses debouncing, but it first checks that the name and codelist is valid before saving?

We could also wait until .altinnstudio is up and running, but not sure how long that is going to take 🤔

@standeren
Copy link
Contributor Author

standeren commented Nov 14, 2024

Nice work! I see that the saving UI pattern here differs from Utforming, where there is no save button, and changes are saved on close. Has this been discussed, or should we consider discussing it?

From what I understood after our meeting last week about saving patterns, I thought this was what we agreed upon 🙈 This is due to the filename has to be provided before the saving can be triggered. So we need a save button to control this interaction. However, when the .altinnstudio file can be taken advantage of, we will not need the save button anymore since we can save the codeList temporarily without a filename in that file 😄

Sorry, it was not on close in Utforming, but on debounce for the manual editor at least 😅

My impression was that we would first try debounce, and if that could not be implemented, then try onBlur. And if that could not be implemented, have a save button. Just a suggestion, but it might be possible to have a handleSave function that uses debouncing, but it first checks that the name and codelist is valid before saving?

We could also wait until .altinnstudio is up and running, but not sure how long that is going to take 🤔

Hm, yes, we could do that 🤔 But what if they add a fileName, add codelist content -> save is triggerd -> and then they change the filename ans the content before they close the modal? Then they will have saved multiple codelists? 🫣

@ErlingHauan
Copy link
Contributor

Hm, yes, we could do that 🤔 But what if they add a fileName, add codelist content -> save is triggerd -> and then they change the filename ans the content before they close the modal? Then they will have saved multiple codelists? 🫣

Good point! The file handling would get more complicated. Then I agree with using a save button 😄

Copy link
Contributor

@TomasEng TomasEng left a comment

Choose a reason for hiding this comment

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

Testet OK.

@TomasEng TomasEng merged commit f2f5101 into main Nov 22, 2024
10 checks passed
@TomasEng TomasEng deleted the 13948-support-for-creating-new-code-list-in-library branch November 22, 2024 12:46
nkylstad pushed a commit that referenced this pull request Nov 26, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for creating new code list in library
3 participants