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

New feature/2066 edit attr modal #2141

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

luistoptal
Copy link
Collaborator

@luistoptal luistoptal commented Dec 23, 2024

Pull request for issue: #2066

IMPORTANT: this is a follow up of #2082, please merge that PR before this one

Wrap the edit attr form in a modal (see comments in #2082 (review)) so that the layout and UX is a bit less cramped and (hopefully) more straightforward, and to easily solve the layout issue expressed in the conversation

How to test?

How have functionalities been implemented?

Wrapped the edit form in a modal that gets rendered with the reponse from the actionEditAttr method of the controller, everything else remains unchanged.

A test was removed since it tested an edge case which is no longer possible. The test checked that a fringe interaction (new attribute and edit attribute being used simultaneously) did not lead to errors. I think the fact that we can remove that test is a good indication at this edge case is no longer possible, as the new layout restricts the user to handle the editted attribute before doing anything else

Any changes to automated tests?

One test was deleted, other tests stay the same as the actions to edit attributes stay the same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant