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

Remove Node Font Size Setting UI #13790

Merged
merged 2 commits into from
Mar 2, 2023
Merged

Conversation

QilongTang
Copy link
Contributor

@QilongTang QilongTang commented Mar 1, 2023

Please Note:

  1. Before submitting the PR, please review How to Contribute to Dynamo
  2. Dynamo Team will meet 1x a month to review PRs found on Github (Issues will be handled separately)
  3. PRs will be reviewed from oldest to newest
  4. If a reviewed PR requires changes by the owner, the owner of the PR has 30 days to respond. If the PR has seen no activity by the next session, it will be either closed by the team or depending on its utility will be taken over by someone on the team
  5. PRs should use either Dynamo's default PR template or one of these other template options in order to be considered for review.
  6. PRs that do not have one of the Dynamo PR templates completely filled out with all declarations satisfied will not be reviewed by the Dynamo team.
  7. PRs made to the DynamoRevit repo will need to be cherry-picked into all the DynamoRevit Release branches that Dynamo supports. Contributors will be responsible for cherry-picking their reviewed commits to the other branches after a LGTM label is added to the PR.

Purpose

per https://jira.autodesk.com/browse/DYN-4891, remove Node Font Size setting from Preferences Panel per customers' request. Customers intend to rely on graph zoom level instead.

Notice there is no effect on serialization because Dynamo does not serialize such a setting currently.
Before:
image

After:
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

remove Node Font Size setting from Preferences Panel per customers' request

Reviewers

@DynamoDS/dynamo

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

@QilongTang QilongTang added this to the 2.18.0 milestone Mar 1, 2023
@avidit
Copy link
Contributor

avidit commented Mar 2, 2023

Can we keep Default Run Settings and Show Run Preview next to one another?

@QilongTang
Copy link
Contributor Author

Can we keep Default Run Settings and Show Run Preview next to one another?

@Jingyi-Wen How do you feel about that?

@Jingyi-Wen
Copy link

Can we keep Default Run Settings and Show Run Preview next to one another?

@Jingyi-Wen How do you feel about that?

I did think about that. From the visual design perspective though - the toggle is only taking one line whereas the radio button for run mode takes 2 lines, so they don't really line up.
Maybe we can switch the position of the Pause splash screen and Run preview so those two are closer

@QilongTang
Copy link
Contributor Author

QilongTang commented Mar 2, 2023

Can we keep Default Run Settings and Show Run Preview next to one another?

@Jingyi-Wen How do you feel about that?

I did think about that. From the visual design perspective though - the toggle is only taking one line whereas the radio button for run mode takes 2 lines, so they don't really line up. Maybe we can switch the position of the Pause splash screen and Run preview so those two are closer

Can we put the toggles on the same line? See below:
image

@Jingyi-Wen
Copy link

Jingyi-Wen commented Mar 2, 2023

Can we keep Default Run Settings and Show Run Preview next to one another?

@Jingyi-Wen How do you feel about that?

I did think about that. From the visual design perspective though - the toggle is only taking one line whereas the radio button for run mode takes 2 lines, so they don't really line up. Maybe we can switch the position of the Pause splash screen and Run preview so those two are closer

Can we put the toggles on the same line?

I wouldn't recommend that. I think putting two unrelated items in the same line could be confusing. In a perfect world, it's better for both the number format and number of recent files should be in their own line too. (I do understand we are trying to save space and avoiding having to scroll to much)

@QilongTang QilongTang requested a review from zeusongit March 2, 2023 16:58
@QilongTang
Copy link
Contributor Author

@Jingyi-Wen @avidit Updated. PTAL

@QilongTang QilongTang merged commit fb00349 into master Mar 2, 2023
@QilongTang QilongTang deleted the RemoveNodeFontSizeSetting branch March 2, 2023 18:00
@QilongTang
Copy link
Contributor Author

Reported regression is sporadic and @RobertGlobant20 is working on fixing it. Merging

sm6srw pushed a commit to sm6srw/Dynamo that referenced this pull request Mar 29, 2023
* Remove Node Font Size Setting UI

* switch order
sm6srw pushed a commit that referenced this pull request Apr 5, 2023
* Remove Node Font Size Setting UI

* switch order
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.

4 participants