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

Edit Python Port Properties #13788

Merged
merged 17 commits into from
Mar 14, 2023
Merged

Conversation

dnenov
Copy link
Collaborator

@dnenov dnenov commented Mar 1, 2023

Purpose

The goal of this PR is to allow users to rename Python imports and outports in order to create more user-friendly and descriptive content for their custom code.

This Figma link is created for visual guidance.

Current WIP

image

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

Release Notes

WIP

  • added a new button to the OutPortContextMenu to allow users to Edit Port Properties
  • for the time being, it is only enabled and visible in case of Python Node Model (in future this check can be removed allowing other nodes to use this functionality as well)

Reviewers

@mjkkirschner
@Amoursol
@sm6srw

FYIs

@Jingyi-Wen

- added a new button to the OutPortContextMenu to allow users to Edit Port Properties
- for the time being, it is only enabled and visible in case of Python Node Model (in future this check can be removed allowing other nodes to use this functionality as well)
@sm6srw sm6srw marked this pull request as ready for review March 1, 2023 15:04
dnenov added 3 commits March 1, 2023 17:30
- At this point, output port can be renamed
- TODO: handle contextmenu - should close after interaction, now keeps alive
- TODO: remove Categories
- TODO: create validation checks (cannot be the same as any other Port, cannot be null, cannot be an Integer, anything else?)
- TODO: Set Prompt name based on Inport/Outport usage
- since we no longer need the popup to stick around after we have launched the Edit Properties Prompt, hide the popup
@sm6srw sm6srw marked this pull request as draft March 2, 2023 17:16
dnenov added 7 commits March 6, 2023 17:12
- added Port Properties Prompt property values as resources
- added a dynamic prompt title based on the PortType value of the port
- further cleaned the UI design
- cannot find DynamoView as an owner for some reason, using WindowStartupLocation.CenterScreen instead
- control cleanup
- refactor chunks of code for better usability
- incorporated number and unique port name validation checks
- now works for both Input and Output ports
- will prompt the user to agree to lose changes when removing port
- changed the validation warning method to status label underneath the Name text box
- disable 'Save' button when we are in a warning state (invalid port name)
- cleans up interaction around blank name
@reddyashish reddyashish marked this pull request as ready for review March 13, 2023 15:05
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.

Conflicts need to be resolved. LGTM with some comments.

dnenov added 6 commits March 14, 2023 14:53
- added comment to public converter
- moved private property to top of class for better readability
- used DynamoUtilities.PathHelper method to check for name validation
- removed unnecessary code
@reddyashish
Copy link
Contributor

@reddyashish reddyashish merged commit ce66b5b into DynamoDS:master Mar 14, 2023
@reddyashish reddyashish added this to the 2.18.0 milestone Mar 14, 2023
sm6srw pushed a commit to sm6srw/Dynamo that referenced this pull request Mar 29, 2023
* Outport - added Edit Port Properties button to port context menu

- added a new button to the OutPortContextMenu to allow users to Edit Port Properties
- for the time being, it is only enabled and visible in case of Python Node Model (in future this check can be removed allowing other nodes to use this functionality as well)

* Output rename working

- At this point, output port can be renamed
- TODO: handle contextmenu - should close after interaction, now keeps alive
- TODO: remove Categories
- TODO: create validation checks (cannot be the same as any other Port, cannot be null, cannot be an Integer, anything else?)
- TODO: Set Prompt name based on Inport/Outport usage

* Hide popup after interaction

- since we no longer need the popup to stick around after we have launched the Edit Properties Prompt, hide the popup

* Resource properties added

- added Port Properties Prompt property values as resources
- added a dynamic prompt title based on the PortType value of the port
- further cleaned the UI design

* WindowsStartupLocation - Screen

- cannot find DynamoView as an owner for some reason, using WindowStartupLocation.CenterScreen instead
- control cleanup

* Code refactor, name validation

- refactor chunks of code for better usability
- incorporated number and unique port name validation checks

* Added Port Properties Prompt to context menu of Input port

- now works for both Input and Output ports

* Added port removal notification message

- will prompt the user to agree to lose changes when removing port

* Validation changed to status label

- changed the validation warning method to status label underneath the Name text box

* Save disabled on warning

- disable 'Save' button when we are in a warning state (invalid port name)
- cleans up interaction around blank name

* Public converter comment

- added comment to public converter

* Moved private property

- moved private property to top of class for better readability

* Used DynamoUtilities method

- used DynamoUtilities.PathHelper method to check for name validation

* Removed commented out code

- removed unnecessary code

* Added back the missing page tag

- fixed page tag
sm6srw pushed a commit that referenced this pull request Apr 5, 2023
* Outport - added Edit Port Properties button to port context menu

- added a new button to the OutPortContextMenu to allow users to Edit Port Properties
- for the time being, it is only enabled and visible in case of Python Node Model (in future this check can be removed allowing other nodes to use this functionality as well)

* Output rename working

- At this point, output port can be renamed
- TODO: handle contextmenu - should close after interaction, now keeps alive
- TODO: remove Categories
- TODO: create validation checks (cannot be the same as any other Port, cannot be null, cannot be an Integer, anything else?)
- TODO: Set Prompt name based on Inport/Outport usage

* Hide popup after interaction

- since we no longer need the popup to stick around after we have launched the Edit Properties Prompt, hide the popup

* Resource properties added

- added Port Properties Prompt property values as resources
- added a dynamic prompt title based on the PortType value of the port
- further cleaned the UI design

* WindowsStartupLocation - Screen

- cannot find DynamoView as an owner for some reason, using WindowStartupLocation.CenterScreen instead
- control cleanup

* Code refactor, name validation

- refactor chunks of code for better usability
- incorporated number and unique port name validation checks

* Added Port Properties Prompt to context menu of Input port

- now works for both Input and Output ports

* Added port removal notification message

- will prompt the user to agree to lose changes when removing port

* Validation changed to status label

- changed the validation warning method to status label underneath the Name text box

* Save disabled on warning

- disable 'Save' button when we are in a warning state (invalid port name)
- cleans up interaction around blank name

* Public converter comment

- added comment to public converter

* Moved private property

- moved private property to top of class for better readability

* Used DynamoUtilities method

- used DynamoUtilities.PathHelper method to check for name validation

* Removed commented out code

- removed unnecessary code

* Added back the missing page tag

- fixed page tag
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