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

feat(Library):adding-tooltip-for-node-category #13857

Merged
merged 2 commits into from
Mar 30, 2023

Conversation

Enzo707
Copy link
Contributor

@Enzo707 Enzo707 commented Mar 29, 2023

tooltips

Purpose

There are three categories of nodes inside Dynamo; Creators, Actions and Queries, which are currently demarcated by a green cross, orange lighting bolt and blue question mark. This PR add tooltips to make it easier to explaining what each of these buckets of nodes does.

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

Now a tooltip will shown over each node category displaying a brief explanation about what does the node bucket do.

Reviewers

@QilongTang

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm what are these changes about new icons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me check that...

}
// document.oncontextmenu = function () {
// return false;
//}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need these lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I forgot to roll this back!

@@ -48,6 +48,7 @@
<Name>DynamoServices</Name>
<Private>False</Private>
</ProjectReference>
<ProjectReference Include="..\PythonMigrationViewExtension\PythonMigrationViewExtension.csproj" />
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this new dependency?

Copy link
Contributor Author

@Enzo707 Enzo707 Mar 29, 2023

Choose a reason for hiding this comment

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

Not sure why this change was added. I didn't change this at least manually. I'm gonna remove that

@Enzo707 Enzo707 force-pushed the DYN-4554-Node-Category-Tooltip branch from ec2a6c3 to 1f7c3d6 Compare March 29, 2023 20:09
@Enzo707 Enzo707 requested a review from QilongTang March 29, 2023 20:09
@@ -24,6 +25,7 @@
using Microsoft.Web.WebView2.Core;
using Microsoft.Web.WebView2.Wpf;
using Newtonsoft.Json;
using ProtoCore.Lang;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these new using valid?

@@ -64,4 +65,4 @@
// You can specify all the values or you can default the Build and Revision Numbers
// by using the '*' as shown below:
// [assembly: AssemblyVersion("1.0.*")]
[assembly: AssemblyFileVersion("2.18.0.2986")]
[assembly: AssemblyFileVersion("2.18.0.4447")]
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually skip the changes to this file, would you revert the changes to this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is the updated minified version based on changes in the other repo

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the use of this additional here, I do not see this got referenced anywhere

Copy link
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

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

Last couple of comments

@Enzo707 Enzo707 force-pushed the DYN-4554-Node-Category-Tooltip branch from 1f7c3d6 to 6393806 Compare March 30, 2023 03:02
@QilongTang QilongTang added this to the 2.18.0 milestone Mar 30, 2023
@QilongTang
Copy link
Contributor

Only sporadic test failure reported from run https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/8776/testReport/. Will merge once basic checks passed, these are triggered because I removed one empty line

@QilongTang QilongTang merged commit 4528ddf into DynamoDS:master Mar 30, 2023
@@ -19,7 +19,7 @@ namespace CoreNodeModels.Properties {
// class via a tool like ResGen or Visual Studio.
// To add or remove a member, edit your .ResX file then rerun ResGen
// with the /str option, or rebuild your VS project.
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "16.0.0.0")]
Copy link
Contributor

Choose a reason for hiding this comment

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

@QilongTang just for curiosity, this change doesn't affect the way the resources are compiled?
I was asking because when I add resources in VS2022 this line is usually updated and the compilation starts to fail
Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

hi @RobertGlobant20 I think v16 belongs to VS2019 and v17 belongs to VS2022. In the past, when we have multiple devs using different versions of VS, there is always this version change back and forth. But I would not expect this impacting compilation result. Do you start to see failure on master now? It builds fine for me on VS2022

Copy link
Contributor

Choose a reason for hiding this comment

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

There are no failures, then if it builds I think we are fine.

sm6srw pushed a commit that referenced this pull request Apr 5, 2023
* feat(Library):adding-tooltip-for-node-category

* Update AssemblySharedInfo.cs

---------

Co-authored-by: Aaron (Qilong) <[email protected]>
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