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 attempt - Removal of deleted content from private list #3030

Merged

Conversation

keikari
Copy link
Contributor

@keikari keikari commented Dec 16, 2023

Issue:
Removing deleted items from private list is bit unclear on desktop, and not possible on mobile.
Also deleted content doesn't look that nice in the list.

Changes:
-Make deleted content look similar to existing content
-Show 3-dot menu for deleted content for clearer deletion option

OLD vs NEW mobile:

2023-12-16_13-43_1
2023-12-16_13-43

OLD vs NEW desktop:

2023-12-16_13-42_1
2023-12-16_13-42

Comment on lines 402 to 403
if ((claim && showNullPlaceholder && shouldHide) || (!claim && playlistPreviewItem)) {
return (
Copy link
Collaborator

@infinite-persistence infinite-persistence Dec 16, 2023

Choose a reason for hiding this comment

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

There are two scenarios for this code block to test. One of the uses in the first scenario (claim && showNullPlaceholder && shouldHide) is the winning claim in Search. The changes will cause blocked winner to have a context menu, which might not make sense.

Before and after PR:
image

I think there are few more users of showNullPlaceholder like channel search, but I didn't try.

A quick way to minimize test coverage is to just spin off another if-block and only apply the changes to playlists.

Copy link
Contributor Author

@keikari keikari Dec 17, 2023

Choose a reason for hiding this comment

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

Does this work? Only should now add the 3-dot menu if playlistPreviewItem.

-Small channel search on wallet send page doesn't seem to be affected at all by these changes.
-Same as above with repost creation search
-Also checked that filtered content in playlist works, but those don't seem to be hidden in preview
-The channel search in channel settings is affected, but looks fine(?)

New | Old
2023-12-17_06-47

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for revising! @infinite-persistence can you take a final look when you get a chance, no rush...

Copy link
Collaborator

@infinite-persistence infinite-persistence left a comment

Choose a reason for hiding this comment

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

👍

@tzarebczan tzarebczan merged commit 4c6114a into OdyseeTeam:master Dec 19, 2023
2 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