RHPAM-1681: Stunner - any change on the canvas scroll process to left-top corner in IE11 #2280
Conversation
|
Jenkins execute full downstream build |
- Applying inherited bug fix for LienzPanel which avoid scroling the parent when focusing - Bug fix for CanvasShortcutsControl - Does not makes sense focusing the canvas on each element registration, neither here, as the canvas could be even not initialized yet (as in IE11)
There was a problem hiding this comment.
@romartin Hi, at first, sorry for the inconvenience I caused. However if we remove this piece of code the keyboard shortcuts wont't work after element is dragged from palette to the canvas. Without this code user has explicitly select the dragged node to get shortcuts working.
I think we have two possibilities:
- Do not support case I described above. I think it is nice to have but the primary goal of shortcuts is to facilitate selenium testing so if you say we won't support it, I am fine.
- Support the case above, it means find place in code that is called just after node is dragged from palette and don't cause the issue on IE11.
If the first option is our choice, I can approve the ensemble. Once more time sorry I caused this inconvenience.
|
thank you for your findings. I guess since it is a blocker for 7.2.0 I think 1-st option (fix IE and don't support append feature for now) is valid for 7.2, but, probably we should consider second option (special case for IE/drop IE support) for further releases. But it depends on how is it difficult to implement second option. If it is an easy fix, probably it will be the best solution. |
|
I tested this PR and it looks and works good, thank you @romartin! Just one more idea about possible fix: is it possible to combine 1 and 2 options? I mean disable append feature only for IE11 since this feature is not critical for users, and there are not so much users of IE11. |
|
Just want to clarify, the append feature itself works, however changes in this ensemble causes it doesn't work immediately after element is dragged form palette to the canvas. After drag operation user has to re-select the element, regardless the browser. |
|
Hey @jomarko no worries at all! I'm just trying to fix it, and good catch! I didn't noticed that dragging from the palette was not focusing the canvas... so lemme try to fix this as well guys. Will let you know in a bit |
|
Just fixed the issue @jomarko mentions when dragging nodes from the palette. Now I think the code is more robust and clear - the idea is that the canvas is being focused once a node is being selected, which happens after dragging a new one from the palette or after using the quick menu (toolbox). This call also happens just once per request. Please this is a blocker, can you review asap!? Thanks in advance! |
|
Jenkins execute full downstream build |
|
Full downstream build fail looks unrelated, but will rerun it to be sure that we won't break build. Jenkins execute full downstream build |
…top corner in IE11 (kiegroup#2280) - Applying inherited bug fix for LienzPanel which avoid scroling the parent when focusing - Bug fix for CanvasShortcutsControl - Does not makes sense focusing the canvas on each element registration, neither here, as the canvas could be even not initialized yet (as in IE11) (cherry picked from commit a1932a9)
Hey @jomarko
Looking at the VCS history I see you created this stuff, so as I'm fixing a bug here, would like you to double check.
The issue is about calling the
canvas.focus()method on eachelementregistration, in theAbstractCanvasShortcutsControlImpl. When opening an existing process in IE11 is causing some issues, as the canvas has not been yet well initialized. Also it's not correct because if you open a process with, let's say 100 elements, it will callfocusalso 100 times...Anyway, probably this call is quite old as the canvas is being focused anyway when removing it. Please can you double check?
It depends on kiegroup/lienzo-core#72
CC @hasys ^^
Thanks in advance!
Roger