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] Fixed re-ordering templates on device add page #434 #829

Merged
merged 3 commits into from
Feb 14, 2024

Conversation

pandafy
Copy link
Member

@pandafy pandafy commented Feb 6, 2024

Closes #434

@pandafy pandafy force-pushed the issues/434-reorder-templates-device-add branch from 59ab2ad to cd31571 Compare February 6, 2024 14:19
@coveralls
Copy link

coveralls commented Feb 6, 2024

Coverage Status

coverage: 99.008%. first build
when pulling 36edf95 on issues/434-reorder-templates-device-add
into ca86ced on master.

@pandafy pandafy force-pushed the issues/434-reorder-templates-device-add branch from cd31571 to e4806e8 Compare February 7, 2024 12:19
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

There seems to be a corner case to handle: if adding device returns a validation error the bug shows up again.

If save is successful the chosen order doesn't seem to be retained, is that because of the bug being fixed in #830? Please double check. It was indeed caused by that issue, please rebase on the latest master.

@pandafy pandafy force-pushed the issues/434-reorder-templates-device-add branch from df756ce to 36edf95 Compare February 13, 2024 11:11
Comment on lines +7 to +9
$(document).on('click', 'a.inline-deletelink', function () {
destroyHiddenSortableWidget($);
});
Copy link
Member Author

Choose a reason for hiding this comment

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

I found that the previous solution was not working when the config form was removed and then added back using the "add row" button.

This solution is also not ideal because it is listening for all .inline-deletelink elements. I had to choose this because inline.js in Django has event.preventDefault present in the click handler.

@nemesifier nemesifier merged commit 87a301a into master Feb 14, 2024
12 checks passed
@nemesifier nemesifier deleted the issues/434-reorder-templates-device-add branch February 14, 2024 19:54
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.

[bug] Re-ordering templates in device add view
3 participants