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

missed some property change notifications to update view #11023

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Aug 19, 2020

Purpose

During testing @smangarole noted that the UI was not being updated for showLabels or IsInput or IsOutput checkboxes when undoing - I found that the tests are passing because the state is actually being correctly undone - but the UI was not being updated.

Two properties were missing and one was misspelled.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

@mjkkirschner mjkkirschner added this to the 2.8 milestone Aug 19, 2020
@QilongTang
Copy link
Contributor

@mjkkirschner I guess the Python engine option is directly binded so it is handled correctly already on view level?

@mjkkirschner
Copy link
Member Author

The PythonNode works because it's bound directly to the property on the NodeModel instead of being bound to a proxy property on the ViewModel.

@QilongTang QilongTang merged commit 498bb7e into DynamoDS:master Aug 19, 2020
@mjkkirschner
Copy link
Member Author

@QilongTang did the tests pass here?

mjkkirschner added a commit to mjkkirschner/Dynamo that referenced this pull request Aug 19, 2020
@QilongTang
Copy link
Contributor

@mjkkirschner The self CI did not get triggered. I only saw the SonarQube scan which I think is not necessary, did I miss something?

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.

2 participants