fix(f6navigation): correct activeElement when shadow root doesn't exist #5910
Conversation
|
|
| if (nextIndex - 1 < 0) { | ||
| nextElement = this.groups[this.groups.length - 1]; |
There was a problem hiding this comment.
0 case was the same as the outer if's else branch
ZsDeak-SAP
force-pushed
the
f6-nav-undefined
branch
from
b7acfe1
to
8452a93
Compare
| if (element.shadowRoot) { | ||
| return this.deepActive(element.shadowRoot); | ||
| deepActive(root) { | ||
| if (root.shadowRoot) { |
There was a problem hiding this comment.
We should check the shadow root of current active element, if shadow root exists.
For me the check should be root.activeElement && root.activeElement.shadowRoot
For example:
Document's active element is web component this check will not handle it.
There was a problem hiding this comment.
I've just been trying out the solution and you are most certainly correct, that's not going to work, updated
| this.groups = getFastNavigationGroups(document.body); | ||
| } | ||
|
|
||
| setSelectedGroup(element) { | ||
| element = this.deepActive(element); | ||
|
|
||
| while (element && element.getAttribute("data-sap-ui-fastnavgroup") !== "true" && element !== document.querySelector("html")) { | ||
| htmlElement = document.querySelector("html"); |
There was a problem hiding this comment.
Since htmlElement will not be overridden we can define it as constant. Please move the definition of the variable at the top of the function and add new line before while loop.
| if (nextIndex - 1 < 0) { | ||
| nextElement = this.groups[this.groups.length - 1]; |
|
Also please check our contribution guidelines for commit messages and please refactor it. |
ZsDeak-SAP
force-pushed
the
f6-nav-undefined
branch
2 times, most recently
from
5ed7972
to
4bb0cdd
Compare
|
@nnaydenow I had tried to read the commit guidelines with care before committing, but it only says to use conventional commits, which I believe I do. If you could please specify which part of the msg I should modify , then I'll happily do so. |
nnaydenow
changed the title
ZsDeak-SAP
force-pushed
the
f6-nav-undefined
branch
from
4bb0cdd
to
ef47b7b
Compare
|
@nnaydenow thanks for the approve 🙏 |
I have found that deepActive won't work if
document.activedoes not have shadowRoot (it's returning undefined), which I suppose is not intended.I am part of SAP CX, please for faster communication reach out in Slack/Teams
My name is Zsolt Deák
Pull Request Checklist