Fix opening side menu on multiple instances #4045
Conversation
|
Your demo site is ready! 🚀 Visit it here: https://fgpv-vpgf.github.io/fgpv-vpgf/multi-instance-sidemenu/samples/index-samples.html |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @yileifeng)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yileifeng)
packages/ramp-core/src/app/ui/sidenav/sidenav.service.js line 325 at r1 (raw file):
$mdSidenav('left') .open() .then(() => $rootElement.find('md-sidenav[md-component-id="left"] button').first().rvFocus());
does the sidenav need an ID? Duplicate ids can mess with screenreaders so if we can avoid that we should.
yileifeng
force-pushed
the
multi-instance-sidemenu
branch
from
5b6b9ba
to
dc0a13e
Compare
There was a problem hiding this comment.
Reviewable status: 1 of 6 files reviewed, 1 unresolved discussion (waiting on @RyanCoulsonCA and @spencerwahl)
packages/ramp-core/src/app/ui/sidenav/sidenav.service.js line 325 at r1 (raw file):
Previously, spencerwahl (Spencer Wahl) wrote…
does the sidenav need an ID? Duplicate ids can mess with screenreaders so if we can avoid that we should.
changed md-component-id to append -{map ID}, let me know if this works
There was a problem hiding this comment.
Reviewable status: 1 of 6 files reviewed, 1 unresolved discussion (waiting on @RyanCoulsonCA and @spencerwahl)
packages/ramp-core/src/app/ui/sidenav/sidenav.service.js line 325 at r1 (raw file):
Previously, yileifeng (Yi Lei Feng) wrote…
changed
md-component-idto append-{map ID}, let me know if this works
if so, can also update the other sidenav ID naming for basemaps (it is also just labeled right currently)
There was a problem hiding this comment.
Reviewable status: 1 of 6 files reviewed, 1 unresolved discussion (waiting on @RyanCoulsonCA, @spencerwahl, and @yileifeng)
packages/ramp-core/src/app/ui/sidenav/sidenav.service.js line 325 at r1 (raw file):
Previously, yileifeng (Yi Lei Feng) wrote…
if so, can also update the other sidenav ID naming for basemaps (it is also just labeled
rightcurrently)
Turns out I misunderstood what kind of id this is and its actually just set as md-component-id which shouldn't cause any issues... my bad for not investigating further.
Can leave these changes in or revert, up to you.
There was a problem hiding this comment.
Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yileifeng)
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yileifeng)
Closes ramp4-pcar4/story-ramp#202
Fixes Storylines issue where page jumps to a previous map (with side menu already open) after opening side menu for a different map on the page. This is due to sidemenu IDs named the same so we end up calling
rvFocuson the first element found. Instead, using$rootElement.find()should find the correct menu. Changes to sample page can be discarded after demo if needed.Demo
This change is