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

Adding output context switch icon for the nodes. #5929

Closed
Tracked by #5899
vitvakatu opened this issue Mar 14, 2023 · 10 comments · Fixed by #6205
Closed
Tracked by #5899

Adding output context switch icon for the nodes. #5929

vitvakatu opened this issue Mar 14, 2023 · 10 comments · Fixed by #6205
Assignees
Labels
d-easy Difficulty: little prior knowledge required p-high Should be completed in the next sprint

Comments

@vitvakatu
Copy link
Contributor

vitvakatu commented Mar 14, 2023

An icon implemented in #5928 should be edited to the graph editor and placed near the nodes, to the right of the visualization switch button. The button should have two states – highlighted and dimmed, and also comes with two icon variants - see #6042.
Integration with (mocked) controllers is in scope.

QA Acceptance Criteria

GIVEN a project opened in the IDE
WHEN The current execution mode is set to `development`
THEN I see the icon variant without the stroke across
WHEN WHEN The current execution mode is set to `production`
THEN I see the icon variant with the stroke across

GIVEN a project opened in the IDE
WHEN I hover over any node
THEN I can see a button next to a visualization switch button
WHEN I click on this button
THEN It changes color with animation (highlights if it was dimmed, dims if it was highlighted)
AND I can see the current state of the button printed in the devtools console

Note: execution modes switching and displaying might not be implemented for this task, in this case only corresponding API for switching the icon variant should be implemented, and the usage demonstrated on interface demo scene.

@vitvakatu vitvakatu added d-easy Difficulty: little prior knowledge required p-high Should be completed in the next sprint g-graph-editor labels Mar 14, 2023
@vitvakatu vitvakatu moved this from ❓New to 📤 Backlog in Issues Board Mar 14, 2023
@vitvakatu vitvakatu added this to the Design Partners milestone Mar 14, 2023
mergify bot pushed a commit that referenced this issue Mar 27, 2023
Implements #5933: adding tooltips to the buttons next to nodes.

To make the UI consistent, I've added tooltips to the `ToggleButton` class directly, since whenever you have an icon button, it seems helpful to have a tooltip.

`ToggleButton` is only used for the profiling button in the top-right corner and the buttons next to nodes. The output context switch button [isn't implemented yet](#5929), but once it is, adding a tooltip should be one-liner.

![Recording 2023-03-22 at 17 21 58](https://user-images.githubusercontent.com/607786/226972920-81033b37-001f-49eb-9fc6-453120f01760.gif)
@Procrat Procrat moved this from 📤 Backlog to 🔧 Implementation in Issues Board Mar 30, 2023
@enso-bot
Copy link

enso-bot bot commented Mar 31, 2023

Stijn Seghers reports a new STANDUP for yesterday (2023-03-30):

Progress: Started implementation. Trying to connect the button to the right action across the FRP networks. It should be finished by 2023-04-03.

@enso-bot
Copy link

enso-bot bot commented Apr 3, 2023

Stijn Seghers reports a new STANDUP for the last Friday (2023-03-31):

Progress: Connected the button to do the right thing. Lost some time trying to figure out why the FRP events weren't reaching the presenter until I realised I was using a demo scene that didn't use those at all. Now, I'm adding a shortcut for switching the execution environment so we can test it properly. It should be finished by 2023-04-03.

@enso-bot
Copy link

enso-bot bot commented Apr 3, 2023

Stijn Seghers reports a new STANDUP for the last Friday (2023-03-31):

Progress: I've added a shortcut for switching the execution environment so we can test it properly. I'm now trying to make the icons switch variants based on the execution environment. It should be finished by 2023-04-03.

@enso-bot
Copy link

enso-bot bot commented Apr 3, 2023

Stijn Seghers reports a new STANDUP for the last Friday (2023-03-31):

Progress: Connected the button to do the right thing. Lost some time trying to figure out why the FRP events weren't reaching the presenter until I realised I was using a demo scene that didn't use those at all. Now, I'm adding a shortcut for switching the execution environment so we can test it properly. It should be finished by 2023-04-03.

@enso-bot
Copy link

enso-bot bot commented Apr 3, 2023

Stijn Seghers reports a new STANDUP for today (2023-04-03):

Progress: I've added a shortcut for switching the execution environment so we can test it properly. I'm now trying to make the icons switch variants based on the execution environment. It should be finished by 2023-04-03.

@Procrat Procrat moved this from 🔧 Implementation to 👁️ Code review in Issues Board Apr 5, 2023
@enso-bot
Copy link

enso-bot bot commented Apr 5, 2023

Stijn Seghers reports a new 🔴 DELAY for yesterday (2023-04-04):

Summary: There is 1 day delay in implementation of the Adding output context switch icon for the nodes. (#5929) task.
It will cause 1 day delay for the delivery of this weekly plan.

Delay Cause: A cumulation of a couple of things: (1) Lost some time trying to figure out why FRP events weren't reaching the presenter until I realised I was using a demo scene that doesn't use the presenter layer at all. (2) The FRP trace command didn't work for some nodes which made it quite difficult to debug why certain events weren't being triggered. I learned that using emit is what broke it. (3) Lost some time trying to put the abstraction that switches icons on the Shape or ShapeView level. (4) Lost some time debugging an issue where the presenter wasn't sending expression updates to the view.

@enso-bot
Copy link

enso-bot bot commented Apr 5, 2023

Stijn Seghers reports a new STANDUP for yesterday (2023-04-04):

Progress: I've made the icons switch variants based on the execution environment. I also connected the on/off state of the button with updates from the presenter and made sure there's only one (temporary) source of truth for the execution environment. It should be finished by 2023-04-04.

@enso-bot
Copy link

enso-bot bot commented Apr 6, 2023

Stijn Seghers reports a new 🔴 DELAY for yesterday (2023-04-05):

Summary: There is 2 day delay in implementation of the Adding output context switch icon for the nodes. (#5929) task.
It will cause 2 day delay for the delivery of this weekly plan.

Delay Cause: Found a bug where updates from the controller weren't reaching the view. During code review, Ilya found a related bug where the button was generated click events when it shouldn't. I'll have a look at that today.

@enso-bot
Copy link

enso-bot bot commented Apr 6, 2023

Stijn Seghers reports a new STANDUP for yesterday (2023-04-05):

Progress: Tracked down a bug where updates from the controller weren't reaching the view. During code review, Ilya found a related bug where the button was generated click events when it shouldn't. I'll have a look at that today It should be finished by 2023-04-06.

@enso-bot
Copy link

enso-bot bot commented Apr 6, 2023

Stijn Seghers reports a new STANDUP for today (2023-04-06):

Progress: Fixed four bugs: (1) the button generated an extra update event when the button state was set explicitly, causing weird behaviour, (2) there was an edge case were the on/off state wasn't set properly, (3) only the output of the latest node was being updated when the execution environment changes, and (4) the view now updates itself instead of waiting for information from the controller. Also addressed some minor PR comments. It should be finished by 2023-04-06.

@vitvakatu vitvakatu moved this from 👁️ Code review to 🌟 Q/A review in Issues Board Apr 11, 2023
@mergify mergify bot closed this as completed in #6205 Apr 11, 2023
mergify bot pushed a commit that referenced this issue Apr 11, 2023
Closes #5929: Adding a node button to enable and disable the output context for that particular node.

I also added a temporary shortcut (cmd-shift-c) to switch the execution environment so we can properly test it.


https://user-images.githubusercontent.com/607786/230036314-052b734a-1846-4057-93d8-2152e1e0cce6.mp4

# Important Notes
While we're waiting to integrate it with the language server, the execution environment is temporarily stored in the presenter. (Otherwise we'd have to define it in multiple places and the behaviour would look rather weird.)

I also fixed a bug where the view didn't get any updates when the context switch expression changed. I'll make a comment on the relevant part. I think the SKIP and FREEZE macros might have the same issue.
@github-project-automation github-project-automation bot moved this from 🌟 Q/A review to 🟢 Accepted in Issues Board Apr 11, 2023
@farmaazon farmaazon moved this from 🟢 Accepted to 🗄️ Archived in Issues Board Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
d-easy Difficulty: little prior knowledge required p-high Should be completed in the next sprint
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants