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
10 stars 13 forks source link

Validate new shortlink input (for save) #8

Closed nazmulidris closed 9 months ago

nazmulidris commented 11 months ago

When new shortlinks are added, make sure to remove any spaces or commas and replace them with _.

amitx13 commented 9 months ago

Hello sir, Can you assign this issue to me?

nazmulidris commented 9 months ago

@amitx13 Thank you so much! 👍🏽 Please let me know if you have any questions! 🙏🏽

amitx13 commented 9 months ago

Hey @nazmulidris what if user itself uses multiple underscores while adding new shortlinks

nazmulidris commented 9 months ago

Hey @nazmulidris what if user itself uses multiple underscores while adding new shortlinks

@amitx13 I think it is ok to keep _ when the user types them. So we should leave them as is. But if there are commands or spaces in the input, then we should replace those with _.

amitx13 commented 9 months ago

@nazmulidris i have solved the issue but in the 2. it doesn't work for _ eg if user input copy foobar,bar then it's works fine it copy the two urls joined with \n but in case of copy foo_bar,bar it unable to copy can u help me with these


  const shortlinkNamesArray = shortlinkNames.split("_");

  // Array to store the URLs for all shortlinks
  const allUrls: string[] = [];

  // Use Promise.all to handle asynchronous operations
  Promise.all(
    shortlinkNamesArray.map((shortlinkName) => {
      return new Promise<void>((resolve, reject) => {
        chrome.storage.sync.get(shortlinkName, (result) => {
          const urls: Urls = result[shortlinkName];
          if (urls === undefined) {
            // Reject with an error message
            reject(`No data found for shortlink: ${shortlinkName}`);
          } else {
            // Check if 'urls' is defined and not an empty array
            if (Array.isArray(urls) && urls.length > 0) {
              allUrls.push(...(urls as string[])); // Add the URLs to the array as a string
            } else {
              reject(`No URLs found for shortlink: ${shortlinkName}`);
            }
            resolve(); // Resolve the promise
          }
        });
      });
    })
  )
    .then(() => {
      if (allUrls.length === 0) {
        showToast("No URLs found for the provided shortlinks", Delays.done, "warning");
      } else {
        const text = allUrls.join("\n");
        copyToClipboard(text);
        showToast(Messages.copyToClipboard, Delays.done, "success");
        //triggerAutoCloseWindowWithDelay();
      }
    })
    .catch((error) => {
      showToast(error, Delays.done, "error");
    });
}```
nazmulidris commented 9 months ago

@amitx13 For the 2nd item, here are some thoughts 🤔 . Instead of the code block you provided, if you try and break it down into smaller chunks, it might be easier to reason about. The following is an attempt to do this; I haven't tested this code :). So it might not compile. Please let me know if you have any issues with this 🙏🏽.

clipboard.ts

In this file, you will find copyShortlinkUrlToClipboard(shortlinkName: string). This function takes one argument which holds whatever the user typed in after the command copy.

  1. So we can modify this function so that it first splits the shortlinkName by , or `, or;`. We can trim any whitespace from each of these strings.
  2. Then we take the array of strings that we get back and those are the actual shortlink names that we were expecting.
  3. We have to make this function async.
export async function copyShortlinkUrlToClipboard(shortlinkNames: string) {
    // More info: https://sl.bing.net/giOzxFaWCWq
    const splitted = shortlinkName.split(/;|,| /)
    const splitted_no_empty = splitted..filter((it) => it.trim() !== "")
    const splitted_trimmed = splitted.map(it => it.trim())

    const urls = [];
    splitted_trimmed.forEach((it) => {
        await _copyShortlinkUrlToClipboard(it, urls) 
    })

    const text = urls.join("\n")
    copyToClipboard(text)

    showToast(Messages.copyToClipboard, Delays.done, "success")
    triggerAutoCloseWindowWithDelay()
}

export async function _copyShortlinkUrlToClipboard(shortlinkName: string, resultArray: string[]) {
   await chrome.storage.sync.get(shortlinkName, (result) => {
    const urls: Urls = result[shortlinkName]
    if (urls === undefined || urls.length === 0) {
      showToast("Please provide a saved shortlink to copy URL(s) from", Delays.done, "warning")
      return
    }

    urls.forEach((it) => {
        resultArray.push(it)
    });
  })
}

popup.tsx

  1. Then we have to modify this file to change the handleEnterKey(...) function, since the copyShortlinkUrlToClipboard() is now async.
    case "copytoclipboard": {
      await copyShortlinkUrlToClipboard(command.shortlinkName)
      return
    }
amitx13 commented 9 months ago

Hey @nazmulidris i have solved the two issue of this project thank you for your help . Actually I am new to this open-source contribution and i came to know about your project from the hecktoberfest Discord can you please help me on how can i merge the code so that it can be counted on my hecktoberfest profile. Once again thank you for the help

nazmulidris commented 9 months ago

@amitx13 Thank you for your help! I really appreciate it. And that is awesome that you are starting your journey into the open-source contributing world! Great job! 👏🏽

I will take a look at your PR tomorrow and make any edits that might need to be made. If I don't have any questions, then I will mark your PR as accepted with the HACKTOBERFEST-ACCEPTED label so that you can get credit for it! 👍🏽