feat(list): disallow selection of archived or pending items #751
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
dskuza
requested review from
nzeltzer and
cyndichin
and removed request for
a team
PocketKit: Coverage: 76.2
Generated by 🚫 Danger Swift against 0dc8c97 |
Gio2018
approved these changes
May 22, 2023
!
Comment on lines
842
to
847
| if networkStatus == .unsatisfied { | ||
| if item.isArchived { | ||
| return !(item.item?.hasArticleComponents ?? false) | ||
| } else { | ||
| return item.isPending | ||
| } | ||
| } | ||
|
|
||
| return item.isPending | ||
| } |
There was a problem hiding this comment.
maybe we can simplify the conditions here with something like this, wdyt?
guard networkStatus == .unsatisfied, item.isArchived else {
return item.isPending
}
return !(item.item?.hasArticleComponents ?? false)
|
|
||
| init(item: ItemsListItem) { | ||
| init(item: ItemsListItem, isDisabled: Bool) { |
There was a problem hiding this comment.
looks like in the existing calls, isDisabled is predominantly set to false: maybe we can add a default value?
There was a problem hiding this comment.
I think that'd clean up the unit tests a lot, specifically (which is mainly where the additions are).
Gio2018
approved these changes
May 22, 2023
added 2 commits
May 22, 2023 16:13
Automatically reloads the current Saves list when the network status changes
dskuza
force-pushed
the
disable-archive-cells
branch
from
bd43cfc to
7046e8a
Compare
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.
Disallows the selection of archived items that contain no offline data, or are pending. Additionally, automatically reloads the current Saves list when the network status changes to show the latest state of the items.
References
IN-1308
Implementation Details
Adds a helper function,
isItemDisabled, that returns true if:This is then used to initialize item presenters appropriately for the required state by adding a
isDisabledargument. This is because determining whether or not an item is disabled required use of network monitoring, which is outside the scope of aSavedItem.Additionally, a network monitor is used to determine when to reload the Saves list as to show the most up-to-date status of items. On monitor change, we send an updated snapshot, reloading the item identifiers representing the visible cells (as to not reload the whole list). This will update the item(s) to their correct coloring.
Test Steps
PR Checklist: