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: only add accordion toggle trigger if in subTables #9

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

Ruesa18
Copy link

@Ruesa18 Ruesa18 commented Sep 20, 2023

I always got the following error in the console.

Error invoking action "click->araise--table-bundle--accordion#toggle"

TypeError: Cannot read properties of null (reading 'classList')
    at extended.toggle (accordion_controller.js:21:1)
    at Binding.invokeWithEvent (stimulus.js:278:1)
    at Binding.handleEvent (stimulus.js:260:1)
    at EventListener.handleEvent (stimulus.js:31:1)

So I poked around and found out that we only use the accordion if there is a sub-table. But we set the trigger regardless. This fix only sets the trigger if there actually are sub-tables that can be opened by the accordion.

@Ruesa18 Ruesa18 self-assigned this Sep 20, 2023
@Ruesa18 Ruesa18 changed the title fix: only add accordion toggle trigger if in subTables exist fix: only add accordion toggle trigger if in subTables Sep 20, 2023
Comment on lines 164 to 166
{% if subTables %}
{{ stimulus_action('araise/table-bundle/accordion', 'toggle', 'click') }}
{% endif %}
Copy link

@renestalder renestalder Sep 21, 2023

Choose a reason for hiding this comment

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

Just saw that by accident.

Coming from a place of having evaluated and dealt with multiple Twig formatters for Twig project standards, I thought it might be useful to know, that having if-else clauses inside HTML Opening/Closing Tags can potentially lead to problems because formatters don't know what to do with it.

Following for example doesn't lead to the same problems:

{% set my_attribute = condition ? stimulus_action(...) : '' %}
<element {{ my_attribute }}></element>

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your input @renestalder. I will update the code accordingly 👍

Copy link
Author

Choose a reason for hiding this comment

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

@renestalder Alright. The changes are made. Would you like to take another look at it? 🙂

@tuxes3 tuxes3 merged commit 8ff5023 into develop Oct 10, 2023
1 check passed
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