fix(web): fix firefox selection issue #708
Conversation
| </label> | ||
| </div> | ||
|
|
||
| {#if showDebugPanel} |
There was a problem hiding this comment.
debug panel is for testing
- if input focus is correct
- if ::selection selector is applied for element outside of iron-remote-gui
| tabindex="0" | ||
| ></canvas> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <style> | ||
| .screen-wrapper { | ||
| .iron-rdp-screen-wrapper { |
There was a problem hiding this comment.
since we do not use shadow-root anymore, in order to reduce css naming conflict, we use iron-rdp prefix
There was a problem hiding this comment.
I’m a bit surprised. When did we stop using a shadow-root?? Was it part of #680 ?
This sounds like a bad idea. Are things working as expected when opening multiple sessions in the same tab?
There was a problem hiding this comment.
works.multi.session.mp4
There was a problem hiding this comment.
Works for me!
There was a problem hiding this comment.
I went over the code, each time a new is created, a new instance of session will be created as well. sperated in memory. From my understanding there's no need for shadow root. However, we can roll it back if you think this is not the practice we want.
There was a problem hiding this comment.
Do you mean "a new instance of ironrdp_web::Session"?
I guess we are fine because we are not trying to fetch the canvas by using its HTML ID.
If it works, then it’s good enough, but my understanding is that using a shadow root is definitely a better practice for a web component.
I don’t think we should have to give prefix to our CSS classes to avoid conflicts with global style sheets… Shadow DOM is what allows proper encapsulation of custom elements.
What is the reason for going away from that in the first place?
irvingoujAtDevolution
requested review from
CBenoit,
ngirot-devolutions,
pacmancoder and
kristahouse
Coverage Report 🤖 ⚙️Past: New: Diff: +0.01% [this comment will be updated automatically] |
|
Thank you, I’ll try on my side too. |
Though I tested it a bit myself, but I'm not 100% confident in this change, would appericate someone can checkout to this branch and test it a liitle.