Hide CommandBarButtonGroupView if all CommandBarButtons are hidden #1254
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
harrieshin
reviewed
Sep 15, 2022
harrieshin
reviewed
Sep 15, 2022
harrieshin
reviewed
Sep 15, 2022
harrieshin
reviewed
Sep 15, 2022
harrieshin
approved these changes
Sep 15, 2022
… hidden - Calls to update the CommandBarButton state now go through the group. This enables the group to be able to check if it needs to hide itself. - Slight modification of the loop used to create a CommadBarButtonGroupView to enable button updates that go through the group. - Updated CommandBarDemoController to hide the "Delete" item instead of the "+" item. Since the "Delete" item is the item in its group, the CommadBarButtonGroupView will have to hide itself when the "Delete" item is hidden, testing the functionality added in this PR.
nodes11
force-pushed
the
HideGroupView
branch
from
1774bb1
to
ecb4edb
Compare
sophialee0416
pushed a commit
to sophialee0416/fluentui-apple
that referenced
this pull request
Sep 20, 2022
…icrosoft#1254) Hide CommandBarButtonGroupView if all contained CommandBarButtons are hidden - The CommandBarItem.propertyChangedUpdateBlock block that calls CommandBarButton.updateState() now takes an additional Bool parameter to indicate if the CommadBarButtonGroupView should also update its state. If the parameter is true the block calls hideGroupIfNeeded() on the group. - Add a call to hideGroupIfNeeded() from CommadBarButtonGroupView. init(buttons:) to ensure the group view is hidden if it is initialized with buttons that are set as hidden. - Modified the loop used to create a CommadBarButtonGroupView so that an update call can be made on the group if needed. - Updated CommandBarDemoController to hide the "Delete" item instead of the "+" item. Since the "Delete" item is the item in its own group, the CommadBarButtonGroupView will have to hide itself when the "Delete" item is hidden, testing the functionality added in this PR.
This was referenced Sep 20, 2022
lcapkovic
pushed a commit
to lcapkovic/fluentui-apple
that referenced
this pull request
Sep 22, 2022
…icrosoft#1254) Hide CommandBarButtonGroupView if all contained CommandBarButtons are hidden - The CommandBarItem.propertyChangedUpdateBlock block that calls CommandBarButton.updateState() now takes an additional Bool parameter to indicate if the CommadBarButtonGroupView should also update its state. If the parameter is true the block calls hideGroupIfNeeded() on the group. - Add a call to hideGroupIfNeeded() from CommadBarButtonGroupView. init(buttons:) to ensure the group view is hidden if it is initialized with buttons that are set as hidden. - Modified the loop used to create a CommadBarButtonGroupView so that an update call can be made on the group if needed. - Updated CommandBarDemoController to hide the "Delete" item instead of the "+" item. Since the "Delete" item is the item in its own group, the CommadBarButtonGroupView will have to hide itself when the "Delete" item is hidden, testing the functionality added in this PR.
Comment on lines
+34
to
+39
| for view in stackView.arrangedSubviews { | ||
| if !view.isHidden { | ||
| allViewsHidden = false | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
swifty way to do this:
isHidden = !stackView.arrangedSubviews.contains(where: { !$0.isHidden })
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Platforms Impacted
Description of changes
Issue:
If all the items in a
CommandBarButtonGroupVieware hidden, the system spacers are still shown in the stack view and create visible empty space. See video below for example.This is a bug with the
CommandBar. Not technically a regression since the ability to hide aCommandBarButtonwas introduced with this commit and this change is fixing a case that was missed with the original commit.Root Cause:
The
CommandBarItem.propertyChangedUpdateBlockwas callingCommandBarButton.updateState()directly to update theisHiddenstate of aCommandBarButton. This works to hide each of the individual buttons, but the superview, theCommadBarButtonGroupView.stackView, isn't aware of when its subviews change their hidden state. If all of the subviews are hidden, the superview needs to also mark itself as hidden so that the system spacers inside the stackView do not remain visible.Fix:
CommandBarItem.propertyChangedUpdateBlockblock that callsCommandBarButton.updateState()now takes an additionalBoolparameter to indicate if theCommadBarButtonGroupViewshould also update its state. If the parameter istruethe block callshideGroupIfNeeded()on the group.hideGroupIfNeeded()fromCommadBarButtonGroupView. init(buttons:)to ensure the group view is hidden if it is initialized with buttons that are set as hidden.CommadBarButtonGroupViewso that an update call can be made on the group if needed.CommandBarDemoControllerto hide the"Delete"item instead of the"+"item. Since the"Delete"item is the item in its own group, theCommadBarButtonGroupViewwill have to hide itself when the"Delete"item is hidden, testing the functionality added in this PR.Verification
Before (Observe space before the Delete item):
Hide.-.Buggy.mov
After (Observe space before the Delete item):
Hide.-.Fixed.mov
Pull request checklist
This PR has considered:
Microsoft Reviewers: Open in CodeFlow