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

bell handling #3316

Closed
jerch opened this issue May 5, 2021 · 3 comments
Closed

bell handling #3316

jerch opened this issue May 5, 2021 · 3 comments
Assignees
Labels
area/api breaking-change Breaks API and requires a major version bump type/enhancement Features or improvements to existing features
Milestone

Comments

@jerch
Copy link
Member

jerch commented May 5, 2021

(Coming from #3314.)

Consolidate bell handling as follows:

  • Strip sound output handling from xterm.js.
  • Propagate onBell as interface to hook into the bell (make sound output an outer env problem). Here integrators can do whatever they want or suits their purpose.
  • Move current direct sound output to a default bell addon, that keeps working in browsers as now, but listens on onBell. If demanded, we can later extend the addon by other bell metaphors, like a visual bell and such.

This cannot be done until next major release version (v5), as it involves changes to the terminal options.

@jerch jerch added this to the 5.0.0 milestone May 5, 2021
@jerch jerch added breaking-change Breaks API and requires a major version bump area/api type/enhancement Features or improvements to existing features labels May 5, 2021
@jerch jerch mentioned this issue Oct 12, 2021
@jerch jerch self-assigned this Oct 22, 2021
@Tyriar
Copy link
Member

Tyriar commented Oct 22, 2021

It might not even be worth having a bell addon but just giving an example sound in the docs? The sound addon one is a different thing imo

@jerch
Copy link
Member Author

jerch commented Oct 25, 2021

Yes a bell-addon on its own is not very useful imho. My thoughts are - remove the sound service in v5 from core repo and just go with the onBell event. From there ppl can hook in whatever they want.

For the sound-addon I need a SoundContext anyway (thats the expensive and limited resource), thus think offering an onBell handler along with the other stuff does not hurt (and its rather small in code size).

@Tyriar
Copy link
Member

Tyriar commented Jul 27, 2022

Pushed to v5 branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api breaking-change Breaks API and requires a major version bump type/enhancement Features or improvements to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants