Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Handle side menu navigation correctly #5503

Merged
merged 23 commits into from
Nov 24, 2020
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions app/components/public/side-menu.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import classic from 'ember-classic-decorator';
import { action, computed } from '@ember/object';
import { computed } from '@ember/object';
import Component from '@ember/component';
import moment from 'moment';
import { SPEAKERS_FILTER } from 'open-event-frontend/routes/public/speakers';
Expand Down Expand Up @@ -45,7 +45,10 @@ export default class SideMenu extends Component {
this.showSessions = this.showSessions || (await this.loader.load(`/events/${this.event.id}/sessions?fields[session]=id&page[size]=1&filter=${JSON.stringify(filters)}`)).data.length;
}

@action
didRender() {
this.scrollToTarget();
}

scrollToTarget() {
document.querySelectorAll('.scroll').forEach(anchor => {
anchor.addEventListener('click', function(e) {
Expand Down
9 changes: 9 additions & 0 deletions app/controllers/public.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,13 @@ export default class PublicController extends Controller {
toggleMenu() {
this.toggleProperty('isMenuOpen');
}

@action
goToLink(section) {
this.transitionToRoute('public.index');
setTimeout(function() {
document.querySelector(`[href='#${section}']`).click();
}, 2000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too much waiting. You should not depend on random waits and timeouts. See if transitionToRoute returns a promise and then await it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamareebjamal now what I found is that I can do

...

transitionToRoute('public.index').promise.then(
   document.querySelector(`[href='#${section}']`).click();
)

... 

but this is not working

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not even valid JS syntax. Of course it won't work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make action async and await on transitionToRoute

Copy link
Member Author

@divyamtayal divyamtayal Nov 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamareebjamal its not working with this also, I have done that only earlier. the function inside then is fired as soon as I click the button not after the next page is rendered

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamareebjamal OK so should I change the time, or 2sec is ok? and if everything else is fine then I think this is done. Let me know anything to change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 sec was not in your first solution.

Then your first solution seems like the best option

Read your first solution again

Copy link
Member Author

@divyamtayal divyamtayal Nov 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamareebjamal So you are talking about my first commit 'Fix Double Click' ? It does not handle going to specific section from diff page but do handle from same function with only one click

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm talking about first working solution which was setTimeout without delay

Copy link
Member Author

@divyamtayal divyamtayal Nov 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamareebjamal so sorry But it was always with 2000ms, I never set setTimeout with 0ms.
Screenshot from 2020-11-08 03-38-17 ??

}

}
18 changes: 9 additions & 9 deletions app/templates/components/public/side-menu.hbs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{{#if (and (not-eq this.session.currentRouteName 'public.cfs.new-session') (not-eq this.session.currentRouteName 'public.cfs.new-speaker') (not-eq this.session.currentRouteName 'public.cfs.edit-speaker') (not-eq this.session.currentRouteName 'public.cfs.edit-session'))}}
<div class="ui fluid vertical {{unless this.device.isMobile 'pointing'}} menu">
{{#if (eq this.session.currentRouteName 'public.index')}}
<a href='#info' {{action "scrollToTarget"}} class='item active scroll'>
<a href='#info' class='item active scroll'>
{{t 'Info'}}
</a>
{{#if this.event.tickets.length}}
<a href='#tickets' {{action "scrollToTarget"}} class='item scroll'>
<a href='#tickets' class='item scroll'>
{{t 'Tickets'}}
</a>
{{/if}}
Expand All @@ -15,11 +15,11 @@
</LinkTo>
{{/if}}
{{else}}
<a class="item" href="{{href-to 'public.index'}}">
<a class="item" {{action this.goToLink 'info'}}>
{{t 'Info'}}
</a>
{{#if this.event.tickets.length}}
<a class="item" href="{{href-to 'public.index'}}#tickets">
<a class="item" {{action this.goToLink 'tickets'}}>
{{t 'Tickets'}}
</a>
{{/if}}
Expand All @@ -46,22 +46,22 @@
{{/if}}
{{#if this.event.isSponsorsEnabled}}
{{#if (eq this.session.currentRouteName 'public.index')}}
<a href='#sponsor' {{action "scrollToTarget"}} class='item scroll'>
<a href='#sponsor' class='item scroll'>
{{t 'Sponsors'}}
</a>
{{else}}
<a class="item" href="{{href-to 'public.index'}}#sponsor">
<a class="item" {{action this.goToLink 'sponsor'}}>
{{t 'Sponsors'}}
</a>
{{/if}}
{{/if}}
{{#if this.event.hasOwnerInfo}}
{{#if (eq this.session.currentRouteName 'public.index')}}
<a href='#owner' {{action "scrollToTarget"}} class='item scroll'>
<a href='#owner' class='item scroll'>
{{t 'Organizer'}}
</a>
{{else}}
<a class="item" href="{{href-to 'public.index'}}#owner">
<a class="item" {{action this.goToLink 'owner'}}>
{{t 'Organizer'}}
</a>
{{/if}}
Expand All @@ -71,7 +71,7 @@
{{t 'Getting here'}}
</a>
{{else}}
<a class="item" href="{{href-to 'public.index'}}#getting-here">
<a class="item" {{action this.goToLink 'getting-here'}}>
{{t 'Getting here'}}
</a>
{{/if}}
Expand Down
4 changes: 2 additions & 2 deletions app/templates/public.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@
<i class="content icon"></i>
</button>
{{#if this.isMenuOpen}}
<Public::SideMenu @class="toggle menu" @event={{this.model}} />
<Public::SideMenu @class="toggle menu" @event={{this.model}} @goToLink={{action 'goToLink'}}/>
{{/if}}
{{/if}}
{{else}}
<div class="ui fluid side-menu">
<Public::SideMenu @event={{this.model}} />
<Public::SideMenu @event={{this.model}} @goToLink={{action 'goToLink'}}/>
</div>
{{/if}}
</div>
Expand Down