r3bl-org / shortlink

This Chrome extension provides a browser action that simply allows the user to create a shortlink for all the selected tabs in the current window. It can be activated via a keyboard shortcut Alt + L. To use it, in the omnibar, type go then press Tab, then type the phrase you have created a shortlink for earlier & it will open the tab(s).
https://chrome.google.com/webstore/detail/shorty/ffhfkgcfbjoadmhdmdcmigopbfkddial?hl=en-US&gl=US
MIT License
11 stars 13 forks source link

Make default action in `popup.tsx` open an existing shortlink (instead of overwriting it) #7

Closed nazmulidris closed 1 year ago

nazmulidris commented 1 year ago

Currently when you type "foo", a new shortlink will be created or an existing one will be updated.

Change the behavior so that if existing shortlink exists, then just open that. Add a "replace / r" command to replace an existing shortlink.

soykot2910 commented 1 year ago

Hey, @nazmulidris, , I'd like to work on this issue. Could you please assign this issue to me?

nazmulidris commented 1 year ago

@soykot2910 Awesome, this is all yours!

nazmulidris commented 1 year ago

@soykot2910 I just went through a major refactor of the code base ("promise-ifying" the chrome.storage API so it is easy to use), so hopefully this will make it easier to work with.

soykot2910 commented 1 year ago

@nazmulidris thanks for the refactor. I'm having trouble understanding it fully. What do you mean by that if existing shortlink exists, then just open that, Also should I automatically add the 'replace' command for users, or should they have the option to use it themselves?

nazmulidris commented 1 year ago

@soykot2910 Thanks! The refactor simplified the code (using async/await and getting rid of callbacks).

The intention behind this issue is to keep users from clobbering an existing shortlink by mistake.

Let's say:

  1. A user creates a shortlink named "foo"
  2. Then the user opens another tab and invokes shortlink, and then types "foo".

Currently this will clobber / overwrite the old value of "foo" with the new one.

A better user experience would be to:

  1. either ask the user for confirmation before overwriting "foo".
  2. or if a shortlink already exists, then
    1. don't overwrite "foo" and instead act as if "go foo" was typed (so this might open the tab(s) for "foo")
    2. add a new command "replace / r" that would allow the user to clobber "foo" explicitly.

Does that make sense? Please let me know if you have any questions 🙏🏽

soykot2910 commented 1 year ago

@nazmulidris thanks for the clarification.

nazmulidris commented 1 year ago

@soykot2910 What do you think of asking the user for confirmation? That could be another way to satisfy the intention of "not clobbering foo" by mistake 👍🏽 .

soykot2910 commented 1 year ago

Yeah, I think that would be more user-friendly.

nazmulidris commented 1 year ago

@soykot2910 Thinking through all the states.

Path 1

  1. If "foo" does not already exist
    • In popup.tsx, Typing "foo" and pressing enter should create a new shortlink
  2. If "foo" already exists
    • In popup.tsx, Typing "foo" and pressing enter should act as if "go foo" was typed and open all the URLs for "foo"
  3. Add a separate command called "replace" (long) / "r" (short) that allows a user to explicitly replace an existing link.

Path 2

Alternatively, we can just prompt the user and ask them for confirmation. It would be great to allow the user to use the keyboard to accept / reject this overwrite operation. If they choose to reject the overwrite, then we can just close the popup.tsx. Optionally: to make the flow more complex, we could then ask the user if they want to name the shortlink something other than "foo".

nazmulidris commented 1 year ago

@soykot2910 I can be convinced to go either way 😄 . Path 1 or 2 both sound good to me. If you have a preference for 1 we can go with that, or 2 if you like that better.

soykot2910 commented 1 year ago

@nazmulidris I'd like to go for option 2.

nazmulidris commented 1 year ago

@soykot2910 Let's go with Option 2 👍🏽 That sounds good to me!

soykot2910 commented 1 year ago

@nazmulidris I want to proceed something like this.

  1. If the shortlink already exists, we first ask the user if they want to overwrite it. If they confirm, we proceed with overwriting. If they decline, we ask if they want to provide a new name.
  2. If the user chooses to provide a new name, we prompt them for it using prompt and save the shortlink with the new name. If they keep the same name or cancel the prompt, we close the popup.
  3. If the user chooses not to provide a new name, we close the popup.
nazmulidris commented 1 year ago

@soykot2910 That flow sounds incredible! Thank you and please let me know if I can help w/ any of it.

soykot2910 commented 1 year ago

Do you have any preference for a branch name? I want to make a pull request.

nazmulidris commented 1 year ago

@soykot2910 Any branch name is good. Looking forward to your PR. Also if you would like to chat about PRs, this is the discord channel for the project: https://discord.gg/UejyhKfx

soykot2910 commented 1 year ago

Here is the PR. I am new to open source, so let me know if I need to change anything.

nazmulidris commented 1 year ago

@soykot2910 Thank you so much for your contribution. Welcome to world of open source contribution; you are doing great 🎉 . I will let you know if I have any comments on the PR 👍🏽

nazmulidris commented 1 year ago

@soykot2910 Fantastic work! My only feedback is to use this feature of github.com to associate a PR with an issue (or vice versa).

https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

I've merged your PR into the main branch and have added the hacktoberfest-accepted label to your PR as well! 🙏🏽

nazmulidris commented 1 year ago

@soykot2910 Also we have a discord server if you would like to chat about PRs & issues: https://discord.gg/UejyhKfx

nazmulidris commented 1 year ago

@soykot2910 Just wanted to let you know that your contribution is already live. I just cut a release for v3.1 of the extension and published it to the Chrome store 🎉

image

If there are other issues you would like to contribute to please let me know! 👍🏽

soykot2910 commented 1 year ago

Wow, that's great news!