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

selection.merge(transition…)? #257

Closed
Fil opened this issue Aug 25, 2020 · 3 comments · Fixed by #285
Closed

selection.merge(transition…)? #257

Fil opened this issue Aug 25, 2020 · 3 comments · Fixed by #285
Labels
idea

Comments

@Fil
Copy link
Member

Fil commented Aug 25, 2020

d3-selection@2 release notes mention:

selection.merge(transition) now throws an error to make it more obvious when you attempt to merge a selection with something that’s not, say because you mistakenly returned a transition from your selection.join’s update function.

However thanks to #248 selection.merge could call transition|selection.selection() before merging.

This would simplify a lot the examples in https://observablehq.com/@d3/selection-join.

svg.selectAll("text")
      .data(randomLetters(), d => d)
      .join(
        enter => enter.append("text")
            .attr("fill", "green")
            .attr("x", (d, i) => i * 16)
            .attr("y", -30)
            .text(d => d)
          .transition(t)
            .attr("y", 0), // 🧨
        update => update
            .attr("fill", "black")
            .attr("y", 0)
          .transition(t)
            .attr("x", (d, i) => i * 16), // 🧨
        exit => exit
            .attr("fill", "brown")
           .transition(t)
            .attr("y", 30)
            .remove()
      );

Note that we can already do:

svg.selectAll("text")
      .data(randomLetters(), d => d)
      .join(
        enter => enter.append("text")
            .attr("fill", "green")
            .attr("x", (d, i) => i * 16)
            .attr("y", -30)
            .text(d => d)
          .transition(t)
            .attr("y", 0)
          .selection(), // 🤘
        update => update
            .attr("fill", "black")
            .attr("y", 0)
          .transition(t)
            .attr("x", (d, i) => i * 16)
          .selection(), // 🤘
        exit => exit
            .attr("fill", "brown")
           .transition(t)
            .attr("y", 30)
            .remove()
      );
@Fil Fil added the idea label Aug 25, 2020
@curran
Copy link
Contributor

curran commented May 24, 2021

I like this line of thinking.

Currently the de facto reference example does the following:

    svg.selectAll("text")
      .data(randomLetters(), d => d)
      .join(
        enter => enter.append("text")
            .attr("fill", "green")
            .attr("x", (d, i) => i * 16)
            .attr("y", -30)
            .text(d => d)
          .call(enter => enter.transition(t)
            .attr("y", 0)),
        update => update
            .attr("fill", "black")
            .attr("y", 0)
          .call(update => update.transition(t)
            .attr("x", (d, i) => i * 16)),
        exit => exit
            .attr("fill", "brown")
          .call(exit => exit.transition(t)
            .attr("y", 30)
            .remove())
      );

I think it does not matter what is returned by exit, so that can already be simplified to:

    svg.selectAll("text")
      .data(randomLetters(), d => d)
      .join(
        enter => enter.append("text")
            .attr("fill", "green")
            .attr("x", (d, i) => i * 16)
            .attr("y", -30)
            .text(d => d)
          .call(enter => enter.transition(t)
            .attr("y", 0)),
        update => update
            .attr("fill", "black")
            .attr("y", 0)
          .call(update => update.transition(t)
            .attr("x", (d, i) => i * 16)),
        exit => exit
            .attr("fill", "brown")
          .transition(t)
            .attr("y", 30)
            .remove()
      );

If selection.merge(transition) did what you suggest here (use the underlying selection), then the example could be simplified to:

    svg.selectAll("text")
      .data(randomLetters(), d => d)
      .join(
        enter => enter.append("text")
            .attr("fill", "green")
            .attr("x", (d, i) => i * 16)
            .attr("y", -30)
            .text(d => d)
          .transition(t)
            .attr("y", 0),
        update => update
            .attr("fill", "black")
            .attr("y", 0)
          .transition(t)
            .attr("x", (d, i) => i * 16),
        exit => exit
            .attr("fill", "brown")
          .transition(t)
            .attr("y", 30)
            .remove()
      );

This would be quite nice.
@curran curran mentioned this issue May 24, 2021
Merged
@curran
Copy link
Contributor

curran commented May 24, 2021

Here's a starting point for a PR that adds this #285
curran added a commit to curran/d3-selection that referenced this issue May 25, 2021
@curran
Alternative solution for d3#257
4928158
@curran curran mentioned this issue May 25, 2021
Merged
@curran
Copy link
Contributor

curran commented May 25, 2021

Here's an alternative idea that enables the same simplification to usage of .join, but does not touch .merge: #286
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea
2 participants
@Fil @curran