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

Implement validation across layouts when loading layout-set, with modal warning user of the issue. #12634

Closed
2 tasks
Tracked by #10857
nkylstad opened this issue Apr 8, 2024 · 11 comments · Fixed by #12988
Closed
2 tasks
Tracked by #10857
Labels
area/ui-editor Area: Related to the designer tool for assembling app UI in Altinn Studio. status/ready-for-dev Status: Used for issues that are ready for development. Has been through grooming. ux UX help needed

Comments

@nkylstad
Copy link
Member

nkylstad commented Apr 8, 2024

Description

Implement a validation when loading a layout-set in the forms editor.

  • Should check for uniqueness of component id across all layouts in the layoutset.

If the layoutset contains duplicate component id's, the user should be given a warning.

This applies only to ids that are duplicate across layouts. If there are duplicate component ids within a layout, this is handled in #12635.

TODO: decide how to inform user:

  • modal that informs on load of layoutset?
  • colors and warning icons on the pages that contain the problematic components?
  • colours and warning icons on the problematic component(s) within a page?

Acceptance criteria

  • Validation is run checking that all component ids across a layoutset are unique.
  • If duplicate component ids are found, user must be informed.
@nkylstad
Copy link
Member Author

nkylstad commented Apr 8, 2024

fyi @Annikenkbrathen

@nkylstad nkylstad added area/ui-editor Area: Related to the designer tool for assembling app UI in Altinn Studio. ux UX help needed labels Apr 8, 2024
@nkylstad nkylstad moved this to 📈 Todo in Team Studio Apr 8, 2024
@Ildest Ildest moved this from 📈 Todo to 👷 In Progress in Team Studio Apr 9, 2024
@Ildest
Copy link
Contributor

Ildest commented Apr 9, 2024

@nkylstad The message needs a way out for the users, what do they have to do? Delete the duplicate?

@Ildest
Copy link
Contributor

Ildest commented Apr 9, 2024

@nkylstad @lassopicasso We need someone to test this more, And describe exactly what happens across layouts, and what the the user must do.

@nkylstad
Copy link
Member Author

@Ildest the user must re-name any duplicates.
In studio today, duplicates across layouts may cause some issues with setting up expressions, but is otherwise not noticeable. But it may also cause issues with evaluating expressions when running the app. So we need to validate and inform the user if they have this problem.

@Annikenkbrathen
Copy link

Annikenkbrathen commented Apr 18, 2024

Here are some updated sketches on this issue

I still have some alternatives that I want feedback on. Personally, I suggest this one. I think it's informative, and I did like this presentation of the ids (in a table) better than the list in an earlier sketch.

Here, the user have two solutions:

  1. Automatically generate new IDs, or
  2. Manually edit the IDs in Studio.

The table above presents the duplicated IDs, along with the corresponding page name and component name.

Skjermbilde 2024-04-18 kl  21 55 54

When the user choose option nr 1, we should consider to inform them that they may loose some connections that are set up in the code. I suggest an " are you sure..." message.
Skjermbilde 2024-04-18 kl  22 00 19

But, If we can have some validations and determine that there are code connections that will be affected, perhaps there's no need for this message. In such cases, we can only present the second solution to the user instead of both.

Skjermbilde 2024-04-18 kl  22 02 59

When you click the on the button, Studio will open the first ID in the first page and component, that is listed up in the table above.
The pages and components should be marked in red som it easy to find. Design suggestion:

Skjermbilde 2024-04-18 kl  22 17 34

In Figma, you can see two other solutions as well
I did also looked into having a general message without showing the IDs that's duplicated, as an easy solution if we think it's difficult to present the information in a good and meaningful way. I believe that the users will be skeptical to auto-generate any IDs without knowing which one we are referring to. So therefore, I think we should show the IDs.

The last suggestion displays the duplicated IDs in a radio button list. The users receive the same information as the examples above, but here they have the option to choose which ID they want to open and start editing. I believe this might become an unnecessary interaction, as I assume it's not easy to choose one from the list in the modal, but easier to locate them manually in the Studio. As long as we navigate to the correct layout set, it should be quite straightforward to find the pages marked in red.

@lassopicasso
Copy link
Contributor

lassopicasso commented Apr 19, 2024

Great work! I also prefer the option you suggested over the other options. You are providing us with two nice alternative solutions here in how we can target the automatically case. (1) it checks for linked connections to this duplicated ID if the user presses "generate new ID's" automatically. Both buttons are displayed if there are no linked connections, and only manual button is displayed when there are connections. (2) if we discover that this is more technically challenging than initially anticipated, the alternative with the popover message is a great fallback.

I also agree to use a table here, and if for some reason there are many duplicated IDs, the table can have a scroll.

I liked the idea of radio buttons that give the user the option to choose which ID they want to edit manually first, but after thinking about the cons; that being unnecessary information with a difficult context, I also proably prefer not to use them.

@Annikenkbrathen Annikenkbrathen removed their assignment Apr 19, 2024
@Annikenkbrathen Annikenkbrathen moved this from 👷 In Progress to 📈 Todo in Team Studio Apr 19, 2024
@Annikenkbrathen Annikenkbrathen moved this to Done 🏁 in Design Altinn 3 Apr 19, 2024
@nkylstad nkylstad added the status/ready-for-specification Status: Used for issues that are ready for functional decription og detailed design. label Apr 22, 2024
@nkylstad nkylstad added status/ready-for-dev Status: Used for issues that are ready for development. Has been through grooming. and removed status/ready-for-specification Status: Used for issues that are ready for functional decription og detailed design. labels May 15, 2024
@JamalAlabdullah JamalAlabdullah moved this from 📈 Todo to 👷 In Progress in Team Studio Jun 5, 2024
@JamalAlabdullah JamalAlabdullah moved this from 👷 In Progress to 🔎 Review in Team Studio Jun 19, 2024
@JamalAlabdullah JamalAlabdullah removed their assignment Jun 19, 2024
@ErlingHauan ErlingHauan self-assigned this Jun 20, 2024
@JamalAlabdullah JamalAlabdullah moved this from 🔎 Review to 👷 In Progress in Team Studio Jun 24, 2024
@JamalAlabdullah JamalAlabdullah moved this from 👷 In Progress to 🔎 Review in Team Studio Jun 26, 2024
@github-project-automation github-project-automation bot moved this from 🔎 Review to 🧪 Test in Team Studio Jun 27, 2024
@framitdavid
Copy link
Collaborator

Tested in dev, but have some feedback to be considered:

  1. Should the ID field have a background color or similar visual indication to highlight it as the field with an error? @Annikenkbrathen any thoughts on this one?

Image

  1. The dialog pops up each time when switching between pages. It should only appear once to prevent annoying the user.

Image

@framitdavid framitdavid moved this from 🧪 Test to 👀 Test feedback in Team Studio Jun 27, 2024
@Annikenkbrathen
Copy link

Annikenkbrathen commented Jun 27, 2024

Tested in dev, but have some feedback to be considered:

  1. Should the ID field have a background color or similar visual indication to highlight it as the field with an error? @Annikenkbrathen any thoughts on this one?

Image

  1. The dialog pops up each time when switching between pages. It should only appear once to prevent annoying the user.

Image

  1. Good catch @framitdavid! I just realized that I did not design a sketch with the ID field closed, but I think, as you suggest, that this field should have the same red background too, yes

  2. hmm I think it might be useful to be reminded every time you load the Lage page, perhaps, but not every time you switch pages in the layoutset, no.

It may be enough that the fields remaining red until its fixed, and that we only show the message once. I am not sure. At the same time I think we should help them, by reminding them to fix the error regularly. And there will always be someone who finds that annoying as well. Sooo.. maybe just try something and see how it works? or maybe we should consider to add a button saying "don't show this message again"

@JamalAlabdullah
Copy link
Contributor

My thoughts are: what if the user closes the modal, does something else on other pages, and then comes back and adds the same duplicate ID? Therefore, I thought to insist on showing the modal in this way until the user fixes it. but maybe we do not need that😊.
@Annikenkbrathen @framitdavid If we agree about these changes , then I can create a new issue to fix them?

@framitdavid
Copy link
Collaborator

I suggest that we add a red background to the ID field, as stated in point 1. We will keep the rest as it is until we receive further feedback or any of our users have other suggestions. Thanks for the great responses @JamalAlabdullah and @Annikenkbrathen! 👏

@JamalAlabdullah
Copy link
Contributor

recolved in this PR:
#13067

@JamalAlabdullah JamalAlabdullah moved this from 👀 Test feedback to ✅ Done in Team Studio Jul 5, 2024
@JamalAlabdullah JamalAlabdullah removed their assignment Jul 5, 2024
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. status/ready-for-dev Status: Used for issues that are ready for development. Has been through grooming. ux UX help needed
Projects
Status: Done 🏁
Archived in project
7 participants