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

add new package manager prefs section #11734

Merged
merged 2 commits into from
Jun 7, 2021

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Jun 4, 2021

Purpose

  • adds a new tab for package manager client settings.

  • adds expanders with empty labels to be filled in.

  • increases width of tabs by 50 pixels.

  • increases width of window by 50pixels.

  • renames some methods to follow c# naming standards.

  • fixes a bug with the labels for geometry scaling- both medium and large had the same description.

  • I would like to make the window resizable horizontally to deal with localized text being longer - but need to check with Morpheus team if that is okay. We're going to wait on this.

Screen Shot 2021-06-04 at 6 35 16 PM

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

add new tab
increase tab width
increase window width
@mjkkirschner mjkkirschner changed the title add new expanders add new package manager prefs section Jun 7, 2021
@QilongTang
Copy link
Contributor

@Jingyi-Wen Can you take a look? The new sub panel tabs looks a bit strange to me, and I wonder if we can make the tabs wider and wordwrap instead of having to make the whole dialog wider? @RobertGlobant20 What do you think?

@Jingyi-Wen
Copy link

@Jingyi-Wen Can you take a look? The new sub panel tabs looks a bit strange to me, and I wonder if we can make the tabs wider and wordwrap instead of having to make the whole dialog wider? @RobertGlobant20 What do you think?

Looking at other tabs we already have, I think we can make the tabs wider for all of them and leave the dialog width as it is

@mjkkirschner
Copy link
Member Author

@Jingyi-Wen if we don't make the dialog wider, then some content wraps in strange ways, let me get you a screenshot.

@mjkkirschner
Copy link
Member Author

mjkkirschner commented Jun 7, 2021

Screen Shot 2021-06-07 at 12 16 14 PM

I'm remembering wrong-
@QilongTang @Jingyi-Wen - here is the dialog with wider tabs, but at 530pixels - it seems something is wrong with the word wrap for the geometry scaling description. It's wrapping, but also looks cutoff. Maybe it just needs a larger margin.

Do you have a preference @Jingyi-Wen? The faster I get this merged, the faster we can move on to other UI for the package manager prefs.

@QilongTang
Copy link
Contributor

Screen Shot 2021-06-07 at 12 16 14 PM

I'm remembering wrong-
@QilongTang @Jingyi-Wen - here is the dialog with wider tabs, but at 530pixels - it seems something is wrong with the word wrap for the geometry scaling description. It's wrapping, but also looks cutoff. Maybe it just needs a larger margin.

Do you have a preference @Jingyi-Wen? The faster I get this merged, the faster we can move on to other UI for the package manager prefs.

@mjkkirschner Maybe we wrap the localization problem into a different task and PR? I feel the impact is larger than we thought?

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.

Looks promising. @mjkkirschner Once comments addressed, LGTM

use existing resource string for installed packages
@@ -62,9 +61,9 @@ private void InitRadioButtonsDescription()
RadioMediumDesc.Inlines.Add(new Run(Res.ChangeScaleFactorPromptDescriptionDefaultSetting) { FontWeight = FontWeights.Bold });
RadioMediumDesc.Inlines.Add(" " + viewModel.OptionsGeometryScale.DescriptionScaleRange[1]);

RadioLargeDesc.Inlines.Add(viewModel.OptionsGeometryScale.DescriptionScaleRange[1]);
RadioLargeDesc.Inlines.Add(viewModel.OptionsGeometryScale.DescriptionScaleRange[2]);
Copy link
Member Author

Choose a reason for hiding this comment

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

geometry scaling descriptions were repeated for medium and large, and extra large was wrong.

@RobertGlobant20
Copy link
Contributor

@Jingyi-Wen Can you take a look? The new sub panel tabs looks a bit strange to me, and I wonder if we can make the tabs wider and wordwrap instead of having to make the whole dialog wider? @RobertGlobant20 What do you think?

@QilongTang I'm not sure about the consequences of making the tabs wider and wordwrap (without making the dialog wider). but as I was guessing if the tabs (and content) width changes we will face several alignment problems like the text in the Geometry Scaling expander.

@mjkkirschner
Copy link
Member Author

I'm going to merge this after one more test run.

Copy link
Contributor

@RobertGlobant20 RobertGlobant20 left a comment

Choose a reason for hiding this comment

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

LGTM, @mjkkirschner could you include a screenshot of the Features and Visual Settingstab after changing the TabPanel.Width

@mjkkirschner
Copy link
Member Author

@RobertGlobant20
Screen Shot 2021-06-07 at 2 12 44 PM
Screen Shot 2021-06-07 at 2 12 32 PM

@mjkkirschner mjkkirschner merged commit 116b3f2 into DynamoDS:master Jun 7, 2021
@mjkkirschner mjkkirschner deleted the managepackprefs branch June 7, 2021 21:32
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.

5 participants