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

Deleting bookmarks #1196

Open
1 of 2 tasks
shtlrs opened this issue Jan 28, 2023 · 8 comments · May be fixed by #1197
Open
1 of 2 tasks

Deleting bookmarks #1196

shtlrs opened this issue Jan 28, 2023 · 8 comments · May be fixed by #1197
Labels
status: planning Discussing details type: feature Relating to the functionality of the application.

Comments

@shtlrs
Copy link
Member

shtlrs commented Jan 28, 2023

I have noticed that sir lancebot only allows us to add bookmarks, but not deleting them.

I figured that this could be useful for whatever reasons people would have w be that the DM channel is too cluttered, or they have bookmarked some message explaining Middlewares for example but then have read a better one which they'd like to replace with the old one.

This should be a pretty easy change considering we'd use add another button to the view we already send in the DMs.

Would you like to implement this yourself?

  • I'd like to implement this feature myself
  • Anyone can implement this feature
@shtlrs shtlrs added status: planning Discussing details type: feature Relating to the functionality of the application. labels Jan 28, 2023
@wookie184
Copy link
Contributor

This can already be done using the .bm delete command in DMs, although I wouldn't be against adding a button (possibly replacing the command).

@shtlrs
Copy link
Member Author

shtlrs commented Jan 28, 2023

This can already be done using the .bm delete command in DMs, although I wouldn't be against adding a button (possibly replacing the command).

I guess it'll be more intuitive through a button, no need to search or even know about the command (Just like I didn't) because it'll be available '' out of box'' now.

And, if for some reason, the interaction fails, then the command will serve as a fallback.

@shtlrs shtlrs linked a pull request Jan 29, 2023 that will close this issue
@ChrisLovering
Copy link
Member

I don't like using a button here. It limits the messages that can be deleted to only new bookmarks, that have the button view in them.

I think this would work better as an app command that has the same behaviour as bm delete (can only delete sir lances messages in your own DMs.)

@shtlrs
Copy link
Member Author

shtlrs commented Jan 29, 2023

Yeah a context command is better to cover the old ones, but that's 2 more clicks for this to be achievable for the new bookmarks.
I think they can both co-live together, as a button jumps straight at you when you see it and I personally think it's a better option for the new bookmarks.

And for the old ones, we can still use either the text command or the context one, WDYT ?

@ChrisLovering
Copy link
Member

Buttons are great for quick access, but there is the trade off with it taking up the a large amount of screen height, which is very prevalent on mobile devices.

SO while I'd agree having buttons for common actions, I just don't see deleting a bookmark to be common enough of an action to warrant a dedicated button with that trade off.

To help with discoverability, I'd also suggest ripping out the current bookmark delete command's functionality, and replacing it with a guide on how to use the context menu.

@shtlrs
Copy link
Member Author

shtlrs commented Jan 31, 2023

Buttons are great for quick access, but there is the trade off with it taking up the a large amount of screen height, which is very prevalent on mobile devices.

SO while I'd agree having buttons for common actions, I just don't see deleting a bookmark to be common enough of an action to warrant a dedicated button with that trade off.

Do we have statistics on the usage of this command ? To be able to determine how "common" it is ?
I understand your point regarding the previous bookmarks, I however struggle to see why not add it for new ones.

To help with discoverability, I'd also suggest ripping out the current bookmark delete command's functionality, and replacing it with a guide on how to use the context menu.

Sure thing, that sounds like a plan.

@ChrisLovering
Copy link
Member

ChrisLovering commented Jan 31, 2023

sir-lance doesn't track command usage like Python bot does, so we can't get that info.

An educated guess is that it's very low, as deletion was only recently added and wasn't announced.

@shtlrs
Copy link
Member Author

shtlrs commented Feb 1, 2023

Alright then sir, I trust your knowledge with Sir Lance

We'll make it a context command & write a guide on how to do it.

Just to be clear, we're talking about a guide in site right ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: planning Discussing details type: feature Relating to the functionality of the application.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants