Set Packageinfo Restart State by QilongTang · Pull Request #9982 · DynamoDS/Dynamo

QilongTang · 2019-09-16T15:27:28Z

Please Note:

Before submitting the PR, please review How to Contribute to Dynamo
Dynamo Team will meet 1x a month to review PRs found on Github (Issues will be handled separately)
PRs will be reviewed from oldest to newest
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
PRs should use either Dynamo's default PR template or one of these other template options in order to be considered for review.
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.
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

DYN-2103
As a Dynamo user, I want workspace dep viewer to set Restart State

gif of whole workflow:

TODO

Add the red bannder
Unit test

Declarations

Check these if you believe they are true

The code base 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.

Reviewers

@DynamoDS/dynamo

FYIs

mjkkirschner · 2019-09-16T21:51:18Z

src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml

                DockPanel.Dock="Top"
                Height="55"
                Margin="1"
-                Visibility="{Binding Path=IsPendingRestart, Mode=OneWay, Converter={StaticResource BoolToVis} }">


have you used the wpf live tree to verify that this dockPanel actually has a dataContext that is not null - you may need to set it explicitly, not on the parent, but on this sub tree.

Yes, I did that. The data context is inherited from parent

@QilongTang you are missing RaisePropertyChanged("IsPendingRestart"); or nameOf(IsPendingRestart) inside your setter for the public property.

mjkkirschner · 2019-09-16T21:51:39Z

src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml.cs

-        /// <summary>
-        /// Property to check if the Dynamo session has package pending for uninstall
-        /// </summary>
-        public Boolean IsPendingRestart { get; set; }


it is potentially time for us to move all this data to a model - right now it's all in the view...

I agree, that should a better example for 3rd party devs, I checked other view extensions, all of them have similar problems

scottmitchell · 2019-09-17T03:37:20Z

src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml

        <!-- Template for the feedback banner -->
        <DockPanel Name="Feedback" VerticalAlignment="Bottom" Grid.Row="2" Width="600">
-            <StatusBar Background="{StaticResource FeedbackSectionBackground}" DockPanel.Dock="Bottom" Height="55">
+            <!-- Restart Banner -->


I'm curious why you all decided to copy the feature preview banner UI for this instead of adding another state icon. Seems like we would want to avoid conflating dependency-state and feature-state UI components.

@scottmitchell The team have discussed around this state, we admit this is not a state for package like the other ones, this is more like a state for Dynamo. So we decide to display this differently and still show it as a warning.

src/WorkspaceDependencyViewExtension/Properties/Resources.en-US.resx

test/DynamoCoreWpfTests/ViewExtensions/WorkspaceDependencyViewExtensionTests.cs

src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml.cs

mjkkirschner · 2019-09-17T18:52:24Z

@QilongTang looks good, just a few comments.

QilongTang · 2019-09-17T19:12:07Z

@mjkkirschner Comments all addressed, I also reverted the last changes in order to preserve the behavior that when a package is marked for uninstall in PM UI, it can also be reflected on package dep viewer ( not binding to the download button). The logic is now back to DependencyRegen() because download package is a common function between regular PM and viewer.

QilongTang · 2019-09-17T19:50:34Z

After talking with @mjkkirschner , Merging

QilongTang added 2 commits September 16, 2019 10:15

Initial Commit

8b02a03

workaround for setting restart state

7c9b946

QilongTang added the WIP label Sep 16, 2019

QilongTang added 2 commits September 16, 2019 17:16

Add restart banner

9b16776

Property binding does not work for me:(

bc3b462

mjkkirschner reviewed Sep 16, 2019

View reviewed changes

Add Unit test

943cf14

QilongTang changed the title ~~[WIP] Set Packageinfo Restart State~~ Set Packageinfo Restart State Sep 16, 2019

QilongTang added PTAL Please Take A Look 👀 and removed WIP labels Sep 16, 2019

scottmitchell reviewed Sep 17, 2019

View reviewed changes

Some clean up, now getter is more consistent

ce4807f

mjkkirschner reviewed Sep 17, 2019

View reviewed changes

src/WorkspaceDependencyViewExtension/Properties/Resources.en-US.resx Outdated Show resolved Hide resolved

mjkkirschner reviewed Sep 17, 2019

View reviewed changes

test/DynamoCoreWpfTests/ViewExtensions/WorkspaceDependencyViewExtensionTests.cs Outdated Show resolved Hide resolved

mjkkirschner reviewed Sep 17, 2019

View reviewed changes

src/WorkspaceDependencyViewExtension/WorkspaceDependencyView.xaml.cs Outdated Show resolved Hide resolved

QilongTang added 3 commits September 17, 2019 15:06

Address Comments

b4b6e2b

Comments

d202fbd

Comments

94beec2

Comments and more tests

12bb1eb

QilongTang merged commit 499bd9b into master Sep 17, 2019

QilongTang deleted the Set-Restart-State branch September 17, 2019 19:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set Packageinfo Restart State#9982

Set Packageinfo Restart State#9982
QilongTang merged 10 commits intomasterfrom
Set-Restart-State

QilongTang commented Sep 16, 2019 •

edited

Loading

Uh oh!

mjkkirschner Sep 16, 2019

Uh oh!

QilongTang Sep 16, 2019

Uh oh!

mjkkirschner Sep 17, 2019 •

edited

Loading

Uh oh!

mjkkirschner Sep 16, 2019

Uh oh!

QilongTang Sep 16, 2019

Uh oh!

scottmitchell Sep 17, 2019 •

edited

Loading

Uh oh!

QilongTang Sep 17, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mjkkirschner commented Sep 17, 2019

Uh oh!

QilongTang commented Sep 17, 2019 •

edited

Loading

Uh oh!

QilongTang commented Sep 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

QilongTang commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Declarations

Reviewers

FYIs

Uh oh!

mjkkirschner Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

QilongTang Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

mjkkirschner Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjkkirschner Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

QilongTang Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

scottmitchell Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QilongTang Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mjkkirschner commented Sep 17, 2019

Uh oh!

QilongTang commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QilongTang commented Sep 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

QilongTang commented Sep 16, 2019 •

edited

Loading

mjkkirschner Sep 17, 2019 •

edited

Loading

scottmitchell Sep 17, 2019 •

edited

Loading

QilongTang commented Sep 17, 2019 •

edited

Loading