Multiple cherrypicks to 2.17.1 #13727
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* standardizing information icons in the preference settings
* Defending code * refining code
* add search sort order
…Fix top menu pixel difference (DynamoDS#13702) * Adjust border to fix 1 pixel anomaly * remove unused login grid * add logic to toggle preview for group * add analytics * Preview > Preview Geometry * ensure that signout option no seen when user sign out
* DYN-4964-WorkingRange-Popup I've added a new button in the Workspace that when is clicked will show a popup containing the working ranges and the one currently selected. This new button will show a different image when the mouse is hover and when is clicked, also I've added a tooltip. For implementing this functionality I've added a new Popup (GeometryScalingPopup.xaml) and it's corresponding ViewModel. Finally I've added a converter that will receive as a parameter the current Working Range and will return a Brush with a specific color so the checkmark will be visible or not. * DYN-4964-WorkingRange-Popup CodeReview1 Added functionality for the new DefaultGeometryScaling property that will be serialized in the DynamoSettings.xml. Also I've disconnected the functionality of selecting the Geometry Scale for the current Workspace in the Preferences panel, now will be selected from the Dynamo workspace and will be serialized in the dyn file (as currently is happening). * DYN-4964-WorkingRange-Popup CodeReview1 Updated the string shown in Preferences panel (Geometry Scaling section) and the tooltip showed when the mouse is over the new Workspace button. * Build Fix When merging master to my branch there were some changed that I didn't noticed in the PR so I'm reverting back those changes. * DYN-4964-WorkingRange-Popup CodeReview 2 Fixed several comments also several methods were removed (like RadioGeometryScaling_Checked method) or moved The property GeoScalingViewModel was moved from DynamoViewModel to WorkspaceViewModel.. The property CurrentGeometryScaling was deleted due that was duplicating a functionality. * DYN-4964-WorkingRange-Popup CodeReview 2 When changing the Workspace Geometry Scaling it was not running the graph so I did some changes so it will be running the graph every time is updated. * DYN-4964-WorkingRange-Popup CodeReview 2 Add functionality for when a custom node is created the Workspace Scale Factor is set. Updating and removing some comments and also I started to add the unit test. * DYN-4964-WorkingRange-Popup CodeReview2 Updating Unit Test * DYN-4964-WorkingRange-Popup Fixing Tests I did the next fixes: The test TestImportCopySettings() was failing due that was reading the DynamoSettings-NewSettings.xml and comparing against the properties in PreferencesSetting so the DefaultScaleFactor was missing in the DynamoSettings-NewSettings.xml file. The test PreferencesGeoScaling_RunGraph_Automatic due that was opening the Preferences panel and changing the Geometry Scaling value for the workspace but now that this value was moved to the Workspace in a Popup then the code needed some changes so we can change the Geometry Scaling value using the Popup.
* DYN-4964-PopupGeoScaling-Bugs Fixing the next bugs reported by Aabi: - The Popup was not closing when clicking in the Dynamo workspace (adding a similar fix like the Workspace ContextMenu). - Adding a shadow over the new Geometry Scaling popup icon. * DYN-4964-WorkingRange-Popup Bug fixes There was a missing case that when the user click a Dynamo menu the Popup was not closing then I added some extra code to close it. * DYN-4964-WorkingRange-Popup Bug Code Review1 Removing extra trailing spaces.
* restrict user login for pkg search * check login state
mjkkirschner
requested changes
Feb 6, 2023
Member
There was a problem hiding this comment.
@zeusongit @avidit @Amoursol @RobertGlobant20 -
there is at least 1 API break in this PR which cannot be merged to 2.17.1.
In addition IMO that the preferences change / geometry scaling factor button new feature work can be omitted from this hotfix. It's a new feature, which does not appear to block any workflows - users can simply use the existing preferences - the code also looks a bit funny to me, though I have not tested it.
Abi also said there are some portions that need some additional work so my suggeston @zeusongit is to remove all cherry picks related to it - this will also fix the API break.
8 tasks
* Markdown procedural image location - fixed hard-coded image location - now identifies image location * Fixed Failing Tests - caught a bug causing issues when no image is present
mjkkirschner
approved these changes
Feb 6, 2023
Member
mjkkirschner
left a comment
There was a problem hiding this comment.
It looks okay, the test is a flaky one I think, but I don't understand the code with toggling group and creating two delegate commands.
This reverts commit 299bbe9.
Contributor
Author
|
@mjkkirschner Fixed the delegate command, why do you think the test is flaky though? |
Member
|
it passes in some of your commits and fails in others - these UI tests seem to have been flaky for the past few months. |
Contributor
Author
Oh..got it |
8 tasks
QilongTang
pushed a commit
that referenced
this pull request
Feb 13, 2023
* DYN-4964-Popup-GeometryScaling Fixes Based in the changes suggested in the next pull request: #13727 I did the next changes: - Remove the property DefaultScaleFactor from IPreferences. - Add braces in some places. - Modify the SetDefaultScaleFactor method for passing one parameter. - Modify the ScaleRange property to use named tuples. - Add comments to public properties. * DYN-4964-Popup-GeometryScaling Bug Fixing bug reported by Aabi about that the button is showing blank when is pressed, then I added the image to be shown when the button is pressed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Dyn 4964 popup geo scaling bugs #13717DYN-4964-WorkingRange-Popup #13656Declarations
Check these if you believe they are true
*.resxfilesFYIs
@mjkkirschner