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

Allow propagation of Defer nested sequence visualizer #1931

Merged
merged 4 commits into from
Jul 24, 2024

Conversation

glopesdev
Copy link
Member

@glopesdev glopesdev commented Jul 20, 2024

Previous enhancements to visualizer propagation rules introduced in #917, #1751 and #1889 allow flexible reuse, modification and encapsulation of visualizers inside group workflows and across subject multicasting.

However, some patterns remain problematic to achieve without introducing additional subject declarations. Specifically, given an observable sequence where data is generated by interaction with a UI element, it is not currently easy to post-process the values in the sequence without either losing the assigned visualizer or introducing a temporary subject.

This PR allows propagation of the visualizer element associated with the nested output in Defer operators, which enables encapsulation of these more flexible constructions with subjects.

For example, by combining Defer with Visualizer, and leveraging the ability to chain VisualizerMapping nodes into the lifted nested visualizer, it becomes possible to solve the case problem discussed in #1896:

Workflow

image

Button (Defer)

image

Visualizer

image
Copy workflow
<?xml version="1.0" encoding="utf-8"?>
<WorkflowBuilder Version="2.8.6-dev0+9f6565675c7a956a370724f1022102862a05eb1d"
                 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                 xmlns:rx="clr-namespace:Bonsai.Reactive;assembly=Bonsai.Core"
                 xmlns:gui="clr-namespace:Bonsai.Gui;assembly=Bonsai.Gui"
                 xmlns="https://bonsai-rx.org/2018/workflow">
  <Workflow>
    <Nodes>
      <Expression xsi:type="rx:Defer">
        <Name>Button</Name>
        <Workflow>
          <Nodes>
            <Expression xsi:type="SubscribeSubject">
              <Name>ButtonPressed</Name>
            </Expression>
            <Expression xsi:type="Combinator">
              <Combinator xsi:type="IntProperty">
                <Value>0</Value>
              </Combinator>
            </Expression>
            <Expression xsi:type="gui:ButtonBuilder">
              <gui:Enabled>true</gui:Enabled>
              <gui:Visible>true</gui:Visible>
              <gui:Text>Click</gui:Text>
            </Expression>
            <Expression xsi:type="rx:PublishSubject">
              <Name>ButtonPressed</Name>
            </Expression>
            <Expression xsi:type="VisualizerMapping" />
            <Expression xsi:type="rx:Visualizer">
              <Workflow>
                <Nodes>
                  <Expression xsi:type="gui:TableLayoutPanelBuilder">
                    <gui:Enabled>true</gui:Enabled>
                    <gui:Visible>true</gui:Visible>
                    <gui:ColumnCount>1</gui:ColumnCount>
                    <gui:RowCount>1</gui:RowCount>
                    <gui:ColumnStyles />
                    <gui:RowStyles />
                    <gui:CellSpans />
                  </Expression>
                  <Expression xsi:type="WorkflowOutput" />
                </Nodes>
                <Edges>
                  <Edge From="0" To="1" Label="Source1" />
                </Edges>
              </Workflow>
            </Expression>
            <Expression xsi:type="WorkflowOutput" />
          </Nodes>
          <Edges>
            <Edge From="0" To="1" Label="Source1" />
            <Edge From="1" To="5" Label="Source1" />
            <Edge From="2" To="3" Label="Source1" />
            <Edge From="3" To="4" Label="Source1" />
            <Edge From="4" To="5" Label="Source2" />
            <Edge From="5" To="6" Label="Source1" />
          </Edges>
        </Workflow>
      </Expression>
      <Expression xsi:type="MemberSelector" />
    </Nodes>
    <Edges>
      <Edge From="0" To="1" Label="Source1" />
    </Edges>
  </Workflow>
</WorkflowBuilder>

Fixes #1896

@glopesdev glopesdev added the feature New planned feature label Jul 20, 2024
@glopesdev glopesdev added this to the 2.9 milestone Jul 20, 2024
@glopesdev
Copy link
Member Author

glopesdev commented Jul 22, 2024

@bruno-f-cruz @PathogenDavid given the discussion today and the realization that it is possible to map other visualizers directly onto the lifted output of the nested Visualizer operator, I don't think we need to do anything more to this PR to resolve this issue.

@PathogenDavid
Copy link
Member

PathogenDavid commented Jul 22, 2024

Do you want to mark this PR as draft then?

(Also good thing you said something because I was about to review it since I thought this PR was slightly separate and still going forward)

@glopesdev
Copy link
Member Author

glopesdev commented Jul 22, 2024

@PathogenDavid actually, it's the contrary, we do want to go forward with this PR 👍 what was happening earlier is that I was wondering whether to add something to it (or make an alternative one) to cover the actual problem raised in the issue, but now we figured out that with just what is in this PR we have everything we need.

The bit with Defer is still important since it allows us to close off the scope around the inner subject, so you can recycle the whole thing inside a single IncludeWorkflow and make your own Button that outputs int instead of a string for example.

P.S.: Just realized it was a slip in my wording that probably threw you off. When I said "I don't think we need to do anything more to this release" I actually meant "I don't think we need to do anything more to this PR".

@glopesdev glopesdev merged commit 4454746 into bonsai-rx:main Jul 24, 2024
10 checks passed
@glopesdev glopesdev deleted the issue-1896 branch July 24, 2024 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New planned feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider allowing Defer to propagate internal sequence visualizer
3 participants