Skip to content

Comments

Fix iframe issue | Dark Mode | Improve UI & input targeting#78

Open
xenos1337 wants to merge 5 commits intodedoussis:mainfrom
xenos1337:main
Open

Fix iframe issue | Dark Mode | Improve UI & input targeting#78
xenos1337 wants to merge 5 commits intodedoussis:mainfrom
xenos1337:main

Conversation

@xenos1337
Copy link

Changes/fixes:

  1. Enable content script in all frames

    • Ensures the extension functions properly within iframes (#76).
  2. Add dark mode switcher

    • Introduced a theme switcher for toggling between light and dark modes (#27).
  3. Migrate options page to popup

    • Redesigned the options page to be built into the popup, providing a more seamless user experience (#77).
  4. Add XPath support for input element targeting

    • Reworked input targeting using XPath to prevent incorrect behavior when switching between input fields.

Tests

I went ahead and tested all features and they work like intented

image
image
image

@xenos1337
Copy link
Author

@dedoussis

@dedoussis
Copy link
Owner

dedoussis commented Jan 22, 2025

Hello @xenos1337,

Huge thanks for submitting this PR and addressing all these issues. This is really appreciated! Since this PR covers an array of different issues and introduces large code changes, would it be possible to split it into separate PRs (ideally 1 PR per each change/fix mentioned in your description)? I would suggest starting with the iframe and XPath fixes, as they're simpler to review and ship, which would help minimize potential rebases between your branches.

The only early, high level feedback I can give right now is:

  • Regarding the Options page: I think we should keep it as a separate page rather than integrating it into the popup. We could simply have the popup's cog icon open the Options in a new tab.
  • Regarding the Help icon: Having it permanently visible in the popup takes up valuable screen real estate. Since the Userguide content is primarily focused on the login process, I think the current approach of showing it only when users are logged out is good enough. The extension's core functionality is quite intuitive for logged-in users, and removing unnecessary UI elements helps keep the popup less overwhelming.
  • I'm directionally happy with the rest of the changes.

@dedoussis-stripe
Copy link
Contributor

Hi @xenos1337! I wanted to fix the iframe issue and was wondering if you were still planning to work on this PR.

@xenos1337
Copy link
Author

Hi @xenos1337! I wanted to fix the iframe issue and was wondering if you were still planning to work on this PR.

I'm sorry yes I will, I'm currently very busy but I'll make sure to get this pushed in the next few days

@dedoussis
Copy link
Owner

@xenos1337 No rush at all! Take your time. I just wanted to check before picking any of those issues up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants