Skip to content

Swipe Action Updates - #489

Merged
kyleve merged 12 commits into
mainfrom
kve/apply-info-in-env
Jul 10, 2023
Merged

Swipe Action Updates#489
kyleve merged 12 commits into
mainfrom
kve/apply-info-in-env

Conversation

@kyleve

@kylevekyleve commented May 19, 2023

Copy link
Copy Markdown
Contributor

A few changes I need for the delete button work in Market:

  • Add Apply*Info to the BlueprintEnvironment: This allows us to magically wire up tapping the remove button exposes the swipe actions in the Market implementation: https://github.com/squareup/market/pull/6261
  • Update some SwipeAction property and typealias names to align better with what each actually does.
  • Replace the true/false flag passed to the swipe action completion with an enum, so it's clear from the callsite what the argument is actually for.

Checklist

Please do the following before merging:

  • Ensure any public-facing changes are reflected in the changelog. Include them in the Main section.

Comment threadBlueprintUILists/Sources/BlueprintHeaderFooterContent.swift
Comment threadBlueprintUILists/Sources/BlueprintItemContent.swift
@kyleve
kyleve requested a review from a teamMay 24, 2023 18:52
@kylevekyleve changed the title Expose apply item info in the Blueprint EnvironmentSwipe Action UpdatesMay 31, 2023
Comment threadBlueprintUILists/Sources/BlueprintHeaderFooterContent.swift Outdated
Comment threadBlueprintUILists/Sources/BlueprintHeaderFooterContent.swift
Comment on lines +287 to +295
guard let swipeActions = self.model.leadingSwipeActions else {
assertionFailure("Cannot showLeadingSwipeActions for `\(self.model.identifier)`, as no swipe actions have been provided.")
return
}

guard swipeActions.actions.isEmpty == false else {
assertionFailure("Cannot showLeadingSwipeActions for `\(self.model.identifier)`, as no swipe actions have been provided.")
return
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🐴

Suggested change
guard let swipeActions =self.model.leadingSwipeActions else{
assertionFailure("Cannot showLeadingSwipeActions for `\(self.model.identifier)`, as no swipe actions have been provided.")
return
}
guard swipeActions.actions.isEmpty ==falseelse{
assertionFailure("Cannot showLeadingSwipeActions for `\(self.model.identifier)`, as no swipe actions have been provided.")
return
}
guard self.model.leadingSwipeActions?.isEmpty ==falseelse{
assertionFailure("Cannot showLeadingSwipeActions for `\(self.model.identifier)`, as no swipe actions have been provided.")
return
}

@kylevekyleveJun 1, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I started with this, but explicitly split it into two asserts, so when hitting the breakpoint, you'd understand if there was a swipe actions config vs if the swipe actions array was empty.

Comment on lines +302 to +310
guard let swipeActions = self.model.trailingSwipeActions else {
assertionFailure("Cannot showTrailingSwipeActions for `\(self.model.identifier)`, as no swipe actions have been provided.")
return
}

guard swipeActions.actions.isEmpty == false else {
assertionFailure("Cannot showTrailingSwipeActions for `\(self.model.identifier)`, as no swipe actions have been provided.")
return
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same hobbyhorse here to take or leave.

/// Pass in `true` to expand the actions (typically only used when deleting the row)
/// or `false` to collapse them.
public typealias CompletionHandler = (_ expandActions: Bool) -> Void
public typealias OnDidPerformAction = (OnDidPerformActionAnimation) -> Void

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor, but nice updates!

Kyle Van Essen added 2 commits July 10, 2023 06:26
* origin/main:
Prep 11.0 release
Paged list layouts now support using bounds, instead of itemInsets.
Prep 10.3.1 release
Update CHANGELOG
iOS 17.0b1 first responder changes
@kyleve

Copy link
Copy Markdown
ContributorAuthor

Note, build is failing due to something around xrOS: XcodesOrg/XcodesApp#401

@kyleve
kyleve merged commit d3aa981 into mainJul 10, 2023
@kyleve
kyleve deleted the kve/apply-info-in-env branch July 10, 2023 15:12
kyleve pushed a commit that referenced this pull request Jul 10, 2023
@n8churn8chur mentioned this pull request Aug 8, 2023
kyleve pushed a commit that referenced this pull request Nov 19, 2023
…rovements
* origin/main: (123 commits)
Update CHANGELOG.md (#508)
Revert "Supplementary Tracking Fixes (#433)"
Revert "Force layout before appear, to avoid animated updates (#505)"
Force layout before appear, to avoid animated updates (#505)
Update workaround versions (#506)
Fix supplementary view + contained first responder reuse issue (#507)
Supplementary Tracking Fixes (#433)
Release 13.0.0 (#504)
Update KeyboardObserver (#499)
CONV-1435: Gravity layout frame change fix - Before: Layout gravity doesn't take into account frame changes. For example, when the orientation changes the scroll position (relative to the bottom) changes - After: Layout gravity takes frame changes into account so the when the frame changes the scroll position relative to the bottom remains unchanged
Release 12.0.0 (#501)
CONV-1435: Add scroll indicator insets to customScrollViewInsets (#500)
CONV-1435: Gravity layout - Adds a new Chat App demo and a new behavior called verticalLayoutGravity. When verticalLayoutGravity is set to bottom, scrolling works the way you would expect for a messaging app.
expose onKeyboardFrameWillChange on ListProperties
onKeyboardFrameWillChange: Improve CHANGELOG, DocC
CONV-1435: Custom keyboard adjustment mode - Adds a .custom KeyboardAdjustmentMode to fully customize inset behavior
remove contentOffset from isContentScrollable calculation, improve comment
Add ListView#isContentScrollable property - Add this property to ListView. It will be used in conjunction with upcoming so-called gravity scrolling changes to workaround an animation issue with paging
Update CI script to reference the `xcodesorg/made/xcodes` package for installing simulator runtimes. (#494)
Swipe Action Updates (#489)
...
Sign up for freeto 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.

2 participants

@kyleve@nononoah