Update charatlas when monitor DPI changes #1172
Conversation
src/Terminal.ts
Outdated
| // Listen for window zoom | ||
| window.addEventListener('resize', () => rendererResizeListener); | ||
| // Listen for monitor DPI change | ||
| window.matchMedia('screen and (-webkit-min-device-pixel-ratio: 1.5)').addListener(rendererResizeListener); |
There was a problem hiding this comment.
Having a look at the -webkit-min-device-pixel-ratio media query, we should be able to replace it with the more standardised min-resolution clause as mentioned in the examples section here: https://developer.mozilla.org/en-US/docs/Web/CSS/@media/-webkit-device-pixel-ratio. I have not tested it yet, but I think it's worth to give it a try as it would also make it work for other browsers.
screen and (-webkit-min-device-pixel-ratio: 1.5)
vs.
screen and (min-resolution: 1.5dppx)
Btw, do you know if using any other value than 1.5 does make a difference?
There was a problem hiding this comment.
I updated this to use a more robust solution that fixes the 1.5 problem 😃
Can try min/max-resolution as well
src/utils/ScreenDprMonitor.ts
Outdated
| } | ||
| // Add listeners for new DPR | ||
| this._currentDevicePixelRatio = window.devicePixelRatio; | ||
| this._minMediaMatchList = window.matchMedia(`screen and (-webkit-min-device-pixel-ratio: ${window.devicePixelRatio})`); |
There was a problem hiding this comment.
I've just tested using this code:
window.matchMedia(`screen and (resolution: ${window.devicePixelRatio}dppx)`).addListener(() => {
console.log('dpi changed')
});It fires whenever that media query match becomes either true or false (good to test by changing the browser zoom level). This means we can listen for the resolution change without having to listen for min and max separately.
src/utils/ScreenDprMonitor.ts
Outdated
| } | ||
| this._listener = listener; | ||
| this._outerListener = () => { | ||
| console.log('change!'); |
|
In general, what do you think about initialising the dpr monitor from within the xterm constructor and then have it emit an event on the terminal instance whenever the screen size changes? The renderer would then listen for that event on the terminal instance instead. This would give the advantage that external addons could also listen for screen size changes without having to create a separate ScreenDprMonitor instance. |
|
I'll refactor as necessary when it's needed. The screen reader support PR needs to listen to this as well so I'll fiddle with it pretty fast #1182. Definitely don't want multiple of them |
|
Pushing this in as tests pass locally and review requests are done. I need to build upon |
Fixes #1118