Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Fix bug to check parent file when meta is checked - #924

Merged
StanleyGoldman merged 10 commits into
masterfrom
fixes/changes-tree-meta-file-checked
Oct 5, 2018
Merged

StanleyGoldman merged 10 commits into
masterfrom
fixes/changes-tree-meta-file-checked

Conversation

@StanleyGoldman

@StanleyGoldman StanleyGoldman commented Oct 3, 2018 •

Copy link
Copy Markdown
Contributor

Fixes #913

As a result of an item having it's check state changed in our tree lists, the check state of it's parents or children might need to be changed.

When performing this action from TreeBase<TNode, TData> we need to be sure to call AddCheckedNode(...) or RemoveCheckedNode(...) when those items are not folders. Those functions are overloaded and allow derived classes to set some additional state.

In order to do this cleanly, all nodes check states are now being changed through a central overloaded method.

private void SetCheckStateOnNode(TNode node, bool setChecked)
{
SetCheckStateOnNode(node, setChecked ? CheckState.Checked : CheckState.Empty);
}
private void SetCheckStateOnNode(TNode node, CheckState setCheckState)
{
var isChecked = setCheckState == CheckState.Checked
|| setCheckState == CheckState.Mixed;
var wasChecked = node.CheckState == CheckState.Checked;
node.CheckState = setCheckState;
if (!node.IsFolder)
{
if (isChecked && !wasChecked)
{
AddCheckedNode(node);
}
else if (!isChecked && wasChecked)
{
RemoveCheckedNode(node);
}
}
}

@queerviolet queerviolet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good!

I think the multilevel test is would be nice, but if you think it'll be too much of a pain / not worth it, it is definitely not a hill I will die on or ask you to die upon.

Comment thread src/GitHub.Api/UI/TreeBase.cs
Comment thread src/GitHub.Api/UI/TreeBase.cs
Comment thread src/GitHub.Api/UI/TreeBase.cs
Comment thread src/tests/UnitTests/UI/TreeBaseTests.cs
@StanleyGoldman

Copy link
Copy Markdown
Contributor Author

@queerviolet 🏆. That was a good call. You made me find two bugs.

@queerviolet queerviolet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yay! I have one more test request, then this looks good to go. 🙏

Comment thread src/GitHub.Api/UI/TreeBase.cs
Comment thread src/GitHub.Api/UI/TreeBase.cs
Comment thread src/tests/UnitTests/UI/TreeBaseTests.cs
@StanleyGoldman
StanleyGoldman merged commit d4eb1f4 into master Oct 5, 2018
@StanleyGoldman
StanleyGoldman deleted the fixes/changes-tree-meta-file-checked branch October 5, 2018 15:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants