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

[DYN-7833] Run All should be disabled in Custom Nodes #86

Conversation

ivaylo-matov
Copy link
Contributor

PR aims to address DYN-7833 and the conversation in this Slack tread

The "Run All" button is now disabled when the workspace is in Custom Node mode. The button is greyed out and a tooltip is displayed to explain that the button is not available in this mode.

DYN-7833-Fix

@QilongTang
@reddyashish
@dnenov

@QilongTang
Copy link
Contributor

QilongTang commented Nov 15, 2024

Thank you @ivaylo-matov I am curious with this code, if user is simply switching back and forth between homeworkspace and custome node workspace, would the execution time data come back after switching from custom node workspace? If not, do we need to cache it after switching?

@ivaylo-matov
Copy link
Contributor Author

@QilongTang , right now it does not.
So, do we want to display default UI in Custom Node mode and then bring back latest result once we are back in the main graph?

@QilongTang
Copy link
Contributor

@ivaylo-matov Switching between workspaces is a pretty normal workflow in Dynamo, I would encourage you to implement caching. The way I see it is that until homeworkspace is closed/ cleared, we dont need to totally wipe out the data when opening custom node workspace, but caching it so we can still reuse when switched back, what do you think?

@ivaylo-matov ivaylo-matov reopened this Nov 19, 2024
Copy link
Contributor

@reddyashish reddyashish left a comment

Choose a reason for hiding this comment

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

LGTM. Will merge once the checks pass.

@reddyashish reddyashish merged commit 2892897 into DynamoDS:master Nov 26, 2024
10 checks 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