cynber / lemmy-instance-assistant

Browser extension to help make Lemmy & Kbin easier to use
GNU General Public License v3.0
55 stars 1 forks source link

Setting multiple home instances #34

Open cynber opened 1 year ago

cynber commented 1 year ago
lewisleedev commented 1 year ago

Can start by implementing it with the right click context menu to allow a user to open a link in a particular instance Can also implement for 'open post in other instance', 'open community in other instance'

I've roughly implemented this feature.

I've only modified m3-background and tested with Chromium as this was a proof of concept that I thought that your approval is needed.

I've decided to use instance list that is already in the settings field as I thought having multiple home instances is kind of redundant when there's already an instances list. There are currently some problems to fix but the main functionality of Left clicking link -> Redirect -> other instances works.

Problems

  1. Cannot create item with duplicate id error handling. I've tested with few options but struggled to find the right approach.
  2. Since "lemmy" | "kbin" type data is newly added to instances list and this is not optional, it need a way to handle pre-existing data. (currently, context menu creation is done inside utils.setAllSettings()).
  3. Duplicate codes

I am willing to actively contribute to this project. Let me know what you think about this implementation. Thank you for your contributions to the lemmy community!

cynber commented 1 year ago

Thank you, this looks great! I agree that using the existing instance list makes sense, and I think the type: "lemmy" should work well. I'll test it out in a few days, but in the meantime I'm going to make a PR into a feature branch and add you as a collaborator. :)

1) This might make more sense once I test it, but what does this refer to?

2) Could we prompt the user to update their settings when they try to use that feature? The function could test the existing data, and then send the user to the settings page if it needs updating.

lewisleedev commented 1 year ago

1.

but what does this refer to?

I'm not sure if I understood correctly but if you're talking about the error, it was a runtime error accessible with runtime.lastError since the context menus aren't really suppose to be created more than once per install which for this functionality is inevitable. I believe we might be fine to just safely suppress the error by using a callback like this:

browserAPI.contextMenus.create({
...
}, () => browserAPI.runtime.lastError);

2.

when they try to use that feature?

Currently when the data type is invalid, no context menus get created. I think we can alternatively create ...Setup instances menu that redirects to settings page instead if datatype is not valid.

Thanks for letting me in as a collaborator!

Edit: I decided to leave as a collaborator as it seems unnessesary and frankly, a bit dangerous. I shouldn't be making direct push or pull any PR anyways. I'm going to make PR whenever necessary. I also strongly advise you to protect at least your main branch. In the meantime, I think something like matrix chatroom would be helpful for further collaborations.

cynber commented 1 year ago
  1. I think that should work! We could also explore how other extensions handle the generation. For example, Bitwarden's menu changes when you navigate to a new tab

  2. Yes that sounds good, it could also handle the case where there is no valid 'home instance' set, such as when the extension is first installed.

Thank you for the notes, I appreciate the guidance! I saw your message on the site, but thank you for adding it here as well. I checked over and updated the branch protection rules. I'll also look into the Matrix chatroom soon :)