googlearchive / app-drawer-template

Application template that demonstrates the PRPL pattern
28 stars 22 forks source link

Drawer does not automatically close when selecting different view in the drawer #20

Open andrewspy opened 8 years ago

andrewspy commented 8 years ago

The view will change in the background, but the drawer does not automatically close, and user need to click on the main view to close the drawer.

cisco78 commented 8 years ago

Just add drawer-toggle to each of the tags in the drawer section / menu list.

cisco78 commented 8 years ago

Just add drawer-toggle to each of the < A > tags in the drawer section / menu list.

cisco78 commented 8 years ago

You are most welcome mate

dandv commented 8 years ago

Actually that's not a good solution. It works in the mobile view, but in desktop mode when the drawer is shown, clicking on any menu item will permanently hide the drawer.

JosefJezek commented 8 years ago

This template is using layout with fixed drawer. Check out this demo https://elements.polymer-project.org/elements/app-layout?view=demo:app-drawer/demo/left-drawer.html&active=app-drawer

Falconerd commented 8 years ago

I'm going to put my solution here as I'm new to Polymer and it took me a while to figure out. Hope that's okay.

Add an on-tap property to the links in the menu:

<a name="view1" on-tap="tappity" href="/view1">View 1</a>

Add a property to the Polymer constructor which matches the event name:

Polymer({
...
  tappity: function() {
    if (onMobile()) {
      this.$.drawer.close();
    }
  },
...
});

function onMobile() {
  // Check if mobile
}

In my case, my onMobile() function just does a regex match to check navigator.userAgent for a bunch of keywords, I haven't field tested it.

I also added an id to the <app-drawer> element which allowed me to grab it via this.$.id.

ansarizafar commented 8 years ago

Drawer must not close on desktop when using drawer-toggle. Is there any official solution for desktop mode?

markus1978 commented 8 years ago

Drawer-toggle always toggles. I think this is a bug. Similar to @Falconerd, I used app-drawer.persistent to check if the drawer should close, before I close it manually. I think the drawer-toggle should only work if the app-drawer is not persistent.

herbalizer404 commented 8 years ago

Same issue, when i add the drawer-toggle attribute, it closes as well. But I don't want it closes on desktop.

addyosmani commented 8 years ago

cc @kevinpschaaf @robdodson Have we since moved app-drawer-template to PSK?

robdodson commented 8 years ago

It's in psk but haven't merged to master yet. It's in the app-drawer-migration branch. Probably merging tomorrow when I get back

On Aug 17, 2016 12:36 PM, "Addy Osmani" notifications@github.com wrote:

cc @kevinpschaaf https://github.com/kevinpschaaf @robdodson https://github.com/robdodson Have we since moved app-drawer-template to PSK?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Polymer/app-drawer-template/issues/20#issuecomment-240470226, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBFDU86BDzX8JclSV8-XFjQ3x4aUCT1ks5qgziggaJpZM4IuL3E .

mercmobily commented 8 years ago

@robdodson I have hit this issue with my app based on PSK2 (from polymer-cli). Am I missing something gigantic (see: there is a simple official fix for this), or shall I start delving into the code with a virtual hammer?

robdodson commented 8 years ago

Yeah I guess this didn't make it into the psk release. The code looks roughly like this:

<iron-selector selected="[[page]]" attr-for-selected="name"
            on-iron-activate="_drawerSelected" role="navigation">
            <a name="home" href="/">Home</a>
            <a name="about" href="/about">About</a>
          </iron-selector>

...

_drawerSelected() {
        if (!this.$.drawer.persistent) {
          this.$.drawer.close();
        }
      }
mercmobily commented 8 years ago

Just my 2c... probably 0.5c! Since this issue is fixed: https://github.com/PolymerElements/app-layout/issues/312 we can now use drawer-toggle safely:

<a drawer-toggle name="home" href="/">Home</a>
<a drawer-toggle name="about" href="/about">About</a>

The only issue with this is that it doesn't "scale". That is, if you want the tag to include more (e.g. an icon and a span), you need to add drawer-toggle to every single element in there.

<a name="home" drawer-toggle href="/page/account/config-info">
  <iron-icon drawer-toggle icon="editor:mode-edit"></iron-icon><span drawer-toggle>About you</span>
</a>

What I did to avoid the drawer-toggle madness was simple:

<a style="position: relative" name="home" drawer-toggle href="/page/account/config-info">
  <div style="position: absolute;top:0;bottom:0;left:0;right:0;" class="drawer-toggler" drawer-toggle></div>
  <iron-icon drawer-toggle icon="editor:mode-edit"></iron-icon><span drawer-toggle>About you</span>
</a>

(I put the extra styles inline for brevity, they are actually added to the class's CSS). This makes sure that the drawer-toggler div always gets the tap. Maybe it's too hacky for a PSK... or maybe not. I prefer it to using the callback, because it's using an existing feature in Polymer.

robdodson commented 8 years ago

this feels like a common enough ask that maybe there's a solution built in to the app drawer elements we haven't discovered yet. @blasten @frankiefu do either of you know of an easy way to ensure the drawer always closes when someone clicks an anchor? Having to mark everything up with attributes feels wrong.

blasten commented 8 years ago

It's also possible to bind to the opened property in app-drawer. That way, you could close the drawer whenever the route has changed. That's how we did it in Shop: https://github.com/Polymer/shop/blob/master/src/shop-app.html#L294

I like this approach because it ensures that the drawer is closed whenever the route has changed regardless of who triggered the action. e.g. nav buttons or a different navigation menu. :)

mercmobily commented 8 years ago

@blasten @robdodson What a great idea!!! This also fixes the issue of drawer-toggle not working with paper-menu (since events are not propagated up) which is a huge bonus. This is definitely the way this should happen in PSK in my opinion -- better than my silly HTML/CSS overlay hack...

mercmobily commented 7 years ago

I just wanted to say that I did exactly this in my app, and it's just about 37492742384 times better. Here is the link to the line of code, regardless of how Master changes:

https://github.com/Polymer/shop/blob/49fc752942f718dbf1d6055bab45bfc95567e2c7/src/shop-app.html#L294

The default template app should use this technique, in my humble opinion!

Merc,

robdodson commented 7 years ago

Glad to hear it. Would you be interested in submitting a PR to the starter-kit with this change?

On Sep 21, 2016 11:56 PM, "Tony Mobily" notifications@github.com wrote:

I just wanted to say that I did exactly this in my app, and it's just about 37492742384 times better. Here is the link to the line of code, regardless of how Master changes:

https://github.com/Polymer/shop/blob/49fc752942f718dbf1d6055bab45bf c95567e2c7/src/shop-app.html#L294

The default template app should use this technique, in my humble opinion!

Merc,

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Polymer/app-drawer-template/issues/20#issuecomment-248827014, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBFDT8ve1QiydLxGtXv_QN1sywUrdvsks5qsiatgaJpZM4IuL3E .

RhysyG commented 7 years ago

Instead of using this.drawerOpened = false; which closes the menu on desktop (even if its in persistent mode) how could you use drawer && !drawer.persistent logic instead?

mercmobily commented 7 years ago

Hi,

Yep that is exactly what I am doing in mine!

Merc.

On 28 Sep 2016 9:14 am, "Rhys" notifications@github.com wrote:

Instead of using this.drawerOpened = false; which closes the menu on desktop (even if its in persistent mode) how could you use drawer && !drawer.persistent logic instead?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Polymer/app-drawer-template/issues/20#issuecomment-250045669, or mute the thread https://github.com/notifications/unsubscribe-auth/ACB7Xi7Fne2QX7aD3L_iszwlvTemPuN2ks5qub-LgaJpZM4IuL3E .

mercmobily commented 7 years ago

@robdodson I would be happy to do a PR for this one. Would you like me to do it? Ready to pounce on it! :dancer:

robdodson commented 7 years ago

Sure thing, just make sure to do it against the starter kit repo

On Wed, Oct 12, 2016 at 8:16 PM, Tony Mobily notifications@github.com wrote:

@robdodson https://github.com/robdodson I would be happy to do a PR for this one. Would you like me to do it? Ready to pounce on it! 💃

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Polymer/app-drawer-template/issues/20#issuecomment-253402652, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBFDawU60P9gxyCIqQ5Un8LYfxq6220ks5qzaKRgaJpZM4IuL3E .

mercmobily commented 7 years ago

I just realised we are in the app-drawer-template repo -- that's why you reminded me to make the changes against the right repo :D Anyhow, https://github.com/PolymerElements/polymer-starter-kit/pull/932 !