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

Setting multiple home instances #34

Open
cynber opened this issue Aug 16, 2023 · 4 comments
Open

Setting multiple home instances #34

cynber opened this issue Aug 16, 2023 · 4 comments
Labels
enhancement New feature or request type: tool Extending functionality

Comments

@cynber
Copy link
Owner

cynber commented Aug 16, 2023

  • Additional settings field, as an array
    • 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'
@cynber cynber added enhancement New feature or request type: tool Extending functionality labels Aug 16, 2023
@lewisleedev
Copy link

lewisleedev commented Sep 21, 2023

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
Copy link
Owner Author

cynber commented Sep 22, 2023

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
Copy link

lewisleedev commented Sep 22, 2023

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);

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
Copy link
Owner Author

cynber commented Sep 27, 2023

  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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request type: tool Extending functionality
Projects
Development

No branches or pull requests

2 participants