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.

Sign up for GitHub

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

Jump to bottom

item-changed firing twice in repeatitem #235

Closed
JoernT opened this issue Nov 25, 2023 · 3 comments · Fixed by #285
Closed

item-changed firing twice in repeatitem #235

JoernT opened this issue Nov 25, 2023 · 3 comments · Fixed by #285
Labels
bug Something isn't working

Comments

@JoernT
Copy link
Contributor

JoernT commented Nov 25, 2023

adding a
<fx-message event="item-changed" target="r-task">Index Changed {event('index')}</fx-message>

to todo2.html example shows the event firing twice instead of once.
@JoernT JoernT added the bug Something isn't working label Nov 25, 2023
@pherk
Copy link

pherk commented Aug 4, 2024

I don't know, whether my observations will help you to improve fx-repeat:

  • if the highlighted item is clicked, the 'item-changed' event will fire too.
  • if the fx-message tag is placed within the template tag, it will fire multiple times (no. of fx-repeatitem elements)
  • the pathologic cases:
    -- if the fx-message tag is placed within the repeat but outside the template, it is only fired once (message and repeat-index correct)
    -- fx-message with fx-action produces an additional strange interaction, if placed outside the template tag: the message is correct but the repeat-index attribute is offset by the number of additional nodes outside template (my guess)

setup from fore-exist, but fore-dev is latest dev

<html lang="en">
<head>
    <meta charset="utf-8"/>
    <meta content="width=device-width, minimum-scale=1, initial-scale=1, user-scalable=yes" name="viewport"/>
    <title>repeat test</title>
    <link href="../resources/css/fore.css" rel="stylesheet"/>
    <style>
        [repeat-index] {
            background-color : yellow;
        }
    </style>
</head>
<body>
    <fx-fore>
        <fx-model>
            <fx-message event="model-construct-done">event model ready</fx-message>
            <fx-instance>
                <data>
                    <sub value="result1"/>
                    <sub value="result2"/>
                    <sub value="result3"/>
                    <sub value="result4"/>
                    <sub value="result5"/>
                    <sub value="result6"/>
                </data>
            </fx-instance>
        </fx-model>
        
        <fx-repeat id="r-sub" ref="sub">
<!--
            <fx-message event="item-changed" target="r-sub">Index Changed(1): {event('index')}</fx-message>
            <fx-action event="item-changed" target="r-sub">
                <fx-message>Index changed(2): {event('index')}</fx-message>
            </fx-action>
            <fx-action event="item-changed" target="r-sub">
                <fx-message>Index changed(3): {event('index')}</fx-message>
            </fx-action>
-->
            <template>
                <fx-message event="item-changed" target="r-sub">Index Changed(4): {event('index')}</fx-message>
                <fx-output ref="./@value"/>
            </template>
        </fx-repeat>
      </fx-fore>
	<script type="module" src="../resources/script/fore-dev.js"/>
  </body>
</html>

@JoernT
Copy link
Contributor Author

JoernT commented Aug 5, 2024 via email

@JoernT JoernT pinned this issue Aug 7, 2024
@DrRataplan DrRataplan mentioned this issue Aug 7, 2024
Merged
@DrRataplan
Copy link
Collaborator

Took a look at this, and I have some observations:

  • Thanks so much for that demo!
  • There used to be a item-changed for the item because of the click, and one because of the focus. These are now one: the repeat-item does not fire an event if the repeat's index is already the current one: no item-changed needed.
  • The indexing in the repeat works on the childList of the fx-repeat. Adding additional elements throws that off. Refactoring that to use the actual sequence from the ref is quite some work, imagine duplicate items in the repeat for example. A repeat over ("a", "b", "c", "a") is OK technically and needed when you take JSON into account, but breaks uniqueness assumptions . Using the fx-repeat here is just more stable. Let's just not place anything else in the childlist of repeats 😉

I made a PR fixing the spammy item-changed events!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix item-changed firing way too often Jinntec/Fore
3 participants
@JoernT @pherk @DrRataplan