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

fix: add alert to ReferenceTab.tsx #14307

Merged
merged 14 commits into from
Jan 3, 2025
Merged

fix: add alert to ReferenceTab.tsx #14307

merged 14 commits into from
Jan 3, 2025

Conversation

Konrad-Simso
Copy link
Contributor

Description

Add alert to ReferenceTab.tsx to inform user of preexisting optionsId.

Related Issue

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)

@Konrad-Simso Konrad-Simso added area/ui-editor Area: Related to the designer tool for assembling app UI in Altinn Studio. kind/chore frontend team/studio-domain1 labels Dec 18, 2024
@github-actions github-actions bot added the solution/studio/designer Issues related to the Altinn Studio Designer solution. label Dec 18, 2024
@Konrad-Simso Konrad-Simso linked an issue Dec 18, 2024 that may be closed by this pull request
@standeren standeren self-assigned this Dec 18, 2024
Copy link

codecov bot commented Dec 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.54%. Comparing base (1b719b9) to head (0c923e2).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14307   +/-   ##
=======================================
  Coverage   95.53%   95.54%           
=======================================
  Files        1860     1860           
  Lines       24094    24104   +10     
  Branches     2779     2783    +4     
=======================================
+ Hits        23019    23029   +10     
  Misses        817      817           
  Partials      258      258           

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

Copy link
Contributor

@standeren standeren left a comment

Choose a reason for hiding this comment

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

Is this PR missing some tests that checks for the alert presence? 🙈
Skjermbilde 2024-12-18 kl  15 43 24

And there is still an issue that it looks like one have added a custom codeliste even though the id belongs to a code list in the library.

And nitpicking, but there are lacking som space between the input field and the alert.

@standeren standeren assigned Konrad-Simso and unassigned standeren Dec 18, 2024
@standeren standeren assigned Konrad-Simso and unassigned standeren Dec 18, 2024
Copy link
Contributor

@standeren standeren left a comment

Choose a reason for hiding this comment

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

Nice!
As mentioned in standup today, there is an issue when typing a custom optionsId and then return to the Kodeliste tab.

Skjermbilde 2025-01-02 kl  08 42 19

And there is and issue when removing the content of the inputfield in the referencetab -> the Kodeliste tab is selected automatically.

@standeren standeren assigned Konrad-Simso and unassigned standeren Jan 2, 2025
@Konrad-Simso
Copy link
Contributor Author

Konrad-Simso commented Jan 2, 2025

The issues you mentioned have existed for a while

One of the issues can be fixed by removing this useEffect in OptionTabs.tsx

  useEffect(() => {
    const updatedSelectedOptionsType = getSelectedOptionsType(
      component.optionsId,
      component.options,
      optionListIds,
    );
    setSelectedOptionsType(updatedSelectedOptionsType);
  }, [optionListIds, component.optionsId, component.options, setSelectedOptionsType]);

It would result in the user not being switched between the tabs unless they click on the titles/buttons at the top.

I noticed a new issue with the changes in this PR. If the user types in a optionsId which already exists in the library it will remove everything which has been written.
A possible solution is to create a useState for the text field and make the initial value based on what if the optionsId is from the library or not

The issue you showed with a picture will be fixed be the redesign PR

@standeren
Copy link
Contributor

The issues you mentioned have existed for a while

One of the issues can be fixed by removing this useEffect in OptionTabs.tsx

  useEffect(() => {
    const updatedSelectedOptionsType = getSelectedOptionsType(
      component.optionsId,
      component.options,
      optionListIds,
    );
    setSelectedOptionsType(updatedSelectedOptionsType);
  }, [optionListIds, component.optionsId, component.options, setSelectedOptionsType]);

It would result in the user not being switched between the tabs unless they click on the titles/buttons at the top.

I noticed a new issue with the changes in this PR. If the user types in a optionsId which already exists in the library it will remove everything which has been written. A possible solution is to create a useState for the text field and make the initial value based on what if the optionsId is from the library or not

The issue you showed with a picture will be fixed be the redesign PR

Okey, nice! Lets create a new issue for it since it has existed for a while? 😊

@standeren standeren assigned Konrad-Simso and unassigned standeren Jan 2, 2025
@standeren standeren removed their assignment Jan 3, 2025
@ErlingHauan ErlingHauan self-assigned this Jan 3, 2025
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.

Testet OK. Ser bra ut 😊

@ErlingHauan
Copy link
Contributor

ErlingHauan commented Jan 3, 2025

Jeg la merke til én ting på tampen som muligens kan fikses i denne PR-en. Denne feilmeldingen skulle hatt litt avstand rundt seg:
bilde
Oppgi navnet på en kodeliste som ikke finnes i referanse-fanen for å få opp denne.

@Konrad-Simso
Copy link
Contributor Author

Jeg la merke til én ting på tampen som muligens kan fikses i denne PR-en. Denne feilmeldingen skulle hatt litt avstand rundt seg: bilde Oppgi navnet på en kodeliste som ikke finnes i referanse-fanen for å få opp denne.

Dette vil bli fikset av feat: update codelist config design

@ErlingHauan ErlingHauan merged commit 20b4540 into main Jan 3, 2025
10 checks passed
@ErlingHauan ErlingHauan deleted the fix/update-referenceTab branch January 3, 2025 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui-editor Area: Related to the designer tool for assembling app UI in Altinn Studio. frontend kind/chore solution/studio/designer Issues related to the Altinn Studio Designer solution. team/studio-domain1
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Merge "select codelist" and "edit codelist" views
3 participants