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

Additional pop up menu options #6106

Merged
merged 11 commits into from
Jun 15, 2021
Merged

Additional pop up menu options #6106

merged 11 commits into from
Jun 15, 2021

Conversation

rafael-xmr
Copy link
Collaborator

@rafael-xmr rafael-xmr commented May 21, 2021

PR Checklist

Please check all that apply to this PR using "x":

  • I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
  • I have checked that this PR does not introduce a breaking change
  • This PR introduces breaking changes and I have provided a detailed explanation below

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting)
  • Refactoring (no functional changes)
  • Documentation changes
  • Other - Please describe:

Fixes

Issue Number: Closes #5782

What is the current behavior?

What is the new behavior?

On own content:

Other information

just missing playlist stuff for now

@rafael-xmr rafael-xmr changed the title Popup Additional pop up menu options May 21, 2021
@rafael-xmr
Copy link
Collaborator Author

Notable new changes:

-Reverted myActiveClaims

-Added some toasts for menu actions:

-Muting and Blocking toast will link to the /block_and_mute settings page, which is a way for the action to be undone since the content being muted or blocked disappears from the page:

-Switched up what Mute and Block messages display on toast, to clear confusion:

-Also changed up redux code a little to separate better Mute from Unmute, and Subscribe from Unsubscribe, like how it already works for Block and Unblock

@rafael-xmr
Copy link
Collaborator Author

Explaining my final solution to claimIsMine:

I added a new selector called makeSelectSigningIsMine, which is similar to makeSelectClaimIsMine but instead it returns true when signing_channel is mine, which then solves the issue of not returning true on the search page:

And also prevents Reposts from returning true, as your claims, which would show on the menu pop up options like "edit" or "delete", and would break trying to edit or delete the reposted content

@tzarebczan
Copy link
Contributor

By search page do you mean the lighthouse results? We can fix that by adding the --include_is_my_output parameter to claim searches and resolves. Looks like we don't pass this in a few places that we should (i.e. homepages, top search result).

I think that's the better way of doing it so it's consistent with the previous method we use for checking ownership.

For claims that are your own, should also hide the mute/block button if it's not muted (allow unmute/unblock if it is).

@rafael-xmr
Copy link
Collaborator Author

I think that's the better way of doing it so it's consistent with the previous method we use for checking ownership.

Hmm I think so too, that one might be it then

For claims that are your own, should also hide the mute/block button if it's not muted (allow unmute/unblock if it is).

Hide mute/block and show unmute/unblock? why not hide both? can you even mute/block your own claims?

@tzarebczan
Copy link
Contributor

Shouldn't allow to block/mute your own, but if someone did, should allow unmuting/blocking.

Pushed on rebased version, before playlists. I can try to take a stab at rebasing on playlists later - unless you want to:?

Some bugs:

  • channel analytics option doesn't load the channel on the analytics page (I see it in the URL, but loads my default).
  • should probably hide options that are already visible on certain pages:
    image

Nice to have:

  • support reposts for deletion button (i.e. I have a repost on my channel page > I can menu > delete.

@jessopb jessopb self-requested a review June 8, 2021 19:21
@rafael-xmr
Copy link
Collaborator Author

rafael-xmr commented Jun 9, 2021

Shouldn't allow to block/mute your own, but if someone did, should allow unmuting/blocking.

✔️

Pushed on rebased version, before playlists. I can try to take a stab at rebasing on playlists later - unless you want to:?

✔️Merged playlists changes

Some bugs:

  • channel analytics option doesn't load the channel on the analytics page (I see it in the URL, but loads my default).

✔️Fixed

  • should probably hide options that are already visible on certain pages:

✔️Now only shows options that are not already on screen

Nice to have:

  • support reposts for deletion button (i.e. I have a repost on my channel page > I can menu > delete.

✔️ Now working

@jessopb
Copy link
Member

jessopb commented Jun 14, 2021

Looking better -
I think it would be better to use the query param that comes from the analytics button and use that to set the active channel on the dash component.
If you just want to ship what you have, strip the analytics item out.
I don't think passing setActiveChanged into the SelectChannel component is the best way.

@jessopb jessopb merged commit c92bcc9 into lbryio:master Jun 15, 2021
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.

Additional pop up menu options
3 participants