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

InputAccordion duplicate elem_id handling #16381

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

w-e-w
Copy link
Collaborator

@w-e-w w-e-w commented Aug 13, 2024

Description

this implements handling of duplicate element IDs for InputAccordion

for more details about this issue read #16373

this PR makes InputAccordion function correctly if the issue described in #16373 were to occur, this would also forcibly modify the element ID by app ending a number
if an extension actually relies on the element ID for a specific use case for example inside JavaScript then the JavaScript will still not function
so this is just a "increase compatibility" but not a fix

extension should not rely on this
the real fixed requires manual modification with the use of #16373


with this PR
InputAccordion now tracks all previously used InputAccordion elem_id and if if it input ID is found to be a duplicate then it append a number at the end of the input id

a on_script_unloaded callback is added to clear / reset the accordion_id_set and global_index

the callback is registered here inside the init amd mpt oput side is becaluse ui_components, happens too early before callbacks can be registered
well if I really want to I can register the call back some other place in the script
but I feel like registering it somewhere else introduces more spaghetti in the code so I decided to put everything inside the class itself

Checklist:

@w-e-w
Copy link
Collaborator Author

w-e-w commented Aug 13, 2024

@light-and-ray I added I've implemented the auto resolving of duplicate elem_id for InputAccordion

@catboxanon
Copy link
Collaborator

#16373 seemed more like the correct fix to me. This isn't needed now, right?

@w-e-w
Copy link
Collaborator Author

w-e-w commented Oct 29, 2024

#16373 seemed more like the correct fix to me. This isn't needed now, right?

not exactly
PR #16373 fixes broken code

this PR makes broken code works correctly by forcibly resolving collisions

#16373 provides a method that fixes the issue so that the issue don't happen, but it does not prevent the issue from happening
this PR resolves the issue if the issue happens,

in practice if an extension did not receive updates and is affected by that issue, it will stop working
this PR maks it makes it work

in my opinion while this PR is not a must, is's still better to have it as it increases compatibility

@catboxanon catboxanon merged commit 5206b93 into dev Oct 29, 2024
6 checks passed
@catboxanon catboxanon deleted the InputAccordion-duplicate-elem_id-handling branch October 29, 2024 15:03
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.

3 participants