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

Add notification and focus options #245

Merged
merged 12 commits into from
Mar 2, 2023
Merged

Conversation

eugenesvk
Copy link
Contributor

@eugenesvk eugenesvk commented Mar 2, 2023

Adds two options:

  1. Show notification messages on (dis)connect (on by default)
  2. Focus browser tab on disconnect (also on by default)

Copy link
Owner

@fregante fregante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! Make sure the tests pass locally as well

source/ghost-text.js Outdated Show resolved Hide resolved
source/ghost-text.js Outdated Show resolved Hide resolved
source/ghost-text.css Outdated Show resolved Hide resolved
source/ghost-text.js Outdated Show resolved Hide resolved
@fregante fregante changed the title feat: Options to suppress connect notifications and refocus on disconnect Options to suppress connect notifications and refocus on disconnect Mar 2, 2023
@eugenesvk
Copy link
Contributor Author

tests also pass locally

source/options.css Outdated Show resolved Hide resolved
source/options.html Outdated Show resolved Hide resolved
source/options.html Outdated Show resolved Hide resolved
Copy link
Owner

@fregante fregante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking…

@fregante
Copy link
Owner

fregante commented Mar 2, 2023

I made some style adjustments. I believe updating webext-base-css will improve it further, but I'll do that separately:

Screenshot 1

Screenshot 2

@fregante fregante changed the title Options to suppress connect notifications and refocus on disconnect Add notification and focus options Mar 2, 2023
@fregante fregante merged commit 3d7dc9f into fregante:main Mar 2, 2023
@fregante
Copy link
Owner

fregante commented Mar 2, 2023

I believe updating webext-base-css will improve it further,

Actually the ugly field is due to Parcel: parcel-bundler/parcel#8866

@fregante
Copy link
Owner

I'm thinking about replacing the "Connected" notification with a non-removable overlay/UI:

Thoughts?

@eugenesvk
Copy link
Contributor Author

eugenesvk commented Mar 14, 2023

I don't like the non-removable part as your other solution is already working great for me

just close the editor tab (which would make more sense, since you're done with it so you want to go back to the browser)

But I agree that for some users it might be better than right-clicking
By the way, what about adding an option to bind a keyboard shortcut to disconnect?

@fregante
Copy link
Owner

fregante commented Mar 15, 2023

I don't like the non-removable part

I suppose I can leave the right click and add the UI as a replacement of the connect notifications. This means you can still hide the UI and continue to use the right click.

By the way, what about adding an option to bind a keyboard shortcut to disconnect?

That's a welcome change and it's easy to implement too. I think it only needs a manifest.json change and a onCommand listener in background.js

PR welcome

There's also a related request to remove the default one (it can still be set though)

#241

@eugenesvk eugenesvk deleted the silence branch March 15, 2023 05:42
@fregante
Copy link
Owner

Done #269

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

Successfully merging this pull request may close these issues.

Option to NOT focus the browser tab on disconnect Option to disable connect/disconnect notifications
2 participants