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

fix: 🐛 [IOSSDKBUG-447]SortFilterItem Reset button func #892

Merged
merged 6 commits into from
Nov 20, 2024

Conversation

restaurantt
Copy link
Collaborator

@restaurantt restaurantt commented Nov 19, 2024

SortFilterItem.PickerItem Reset button functionality.

  1. Provide reset button configuration to customize hidden, title, type.
  2. Selected section add deselect all.
  3. Selected in header add count 'Selected (2)' to show, default is showing count.
  4. update select all showing logic.
    Simulator Screenshot - iPhone 16 Pro Max - 2024-11-20 at 11 07 26
    Simulator Screenshot - iPhone 16 Pro Max - 2024-11-20 at 11 07 06

SortFilterItem.PickerItem Reset button functionality
SortFilterItem.PickerItem Reset button functionality
@restaurantt restaurantt requested a review from a team as a code owner November 19, 2024 08:42
@restaurantt restaurantt requested review from billzhou0223 and removed request for a team November 19, 2024 08:42
@dyongxu dyongxu requested a review from xiaoyu0722 November 20, 2024 01:57
Spacer()
self.resetAction.accessibilityIdentifier("Reset")
}
})
Copy link
Contributor

Choose a reason for hiding this comment

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

What about long title if you put title and others in one ZStack?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Haven't find a good solution for now. Try later.

@@ -346,6 +346,37 @@ extension SortFilterItem {
}
}

/// FilterFeedbackBar ResetButton Configuration
public struct FilterFeedbackBarResetButtonConfiguration: Equatable {
Copy link
Contributor

Choose a reason for hiding this comment

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

Name confusion, you can try FilterFeedbackRightBarButtonConfiguration.

Copy link
Contributor

Choose a reason for hiding this comment

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

@xiaoyu0722 I think we should not use the wording "right" or "left" in the API names, since it will be confusing in the "right to left" language. It looks ok to me for the current name, but pls double check with Jiajun if that is actually what he wants

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's a reset button, but it will also work for clear all, that's what makes me confused.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Still use FilterFeedbackBarResetButtonConfiguration.

Copy link
Contributor

@dyongxu dyongxu left a comment

Choose a reason for hiding this comment

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

In the test case for "List Multiple", click "Select All", then click it again, all selected values are cleared. I would expect the second click will not trigger any clear action.

@restaurantt
Copy link
Collaborator Author

In the test case for "List Multiple", click "Select All", then click it again, all selected values are cleared. I would expect the second click will not trigger any clear action.

Fixed.

@dyongxu dyongxu merged commit 472b6d8 into SAP:main Nov 20, 2024
12 checks passed
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.

3 participants