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

fix: Handle side menu navigation correctly #5503

merged 23 commits into from
Nov 24, 2020

Conversation

divyamtayal
Copy link
Member

@divyamtayal divyamtayal commented Nov 6, 2020

Fixes #5123

Short description of what this resolves:

Require two clicks for moving to same section eg. sponsors, tickets etc.

Changes proposed in this pull request:

  • Since the problem what I found is that eventListeners were no longer there for each button until an unless we click on any button.
  • EventListeners need to be with each button before, so I have resolved this by adding life cycle Hook
  • Now it requires only one click as normal to move to diff sections of the page.
  • Going to sponsors, getting here etc from speaker, call for speaker etc pages were not exactly taking to the section of page so fixed this.
  • Also clicking sponsors, organizers etc from diff page gets to public.index page but the info button remain active so fixed this.

TODO

  • Single click and go to section on same page
  • Target to specific section from diff page.

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@vercel
Copy link

vercel bot commented Nov 6, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/8polg26o6
✅ Preview: https://open-event-frontend-git-double-click-5123.eventyay.vercel.app

@divyamtayal
Copy link
Member Author

@iamareebjamal Please see to the changes I have made in it and let me know anything to do.

@divyamtayal divyamtayal changed the title Double click 5123 Fix: To get to sections on the same page that are listed on the menu requires two clicks Nov 6, 2020
@iamareebjamal
Copy link
Member

Go to https://open-event-frontend-i18x7pe1p.vercel.app/e/1f720482/speakers
Click on sponsors
Not working

@lgtm-com
Copy link

lgtm-com bot commented Nov 6, 2020

This pull request introduces 1 alert when merging cb22baf into 79e0cbd - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@divyamtayal
Copy link
Member Author

divyamtayal commented Nov 6, 2020

https://open-event-frontend-i18x7pe1p.vercel.app/e/1f720482/speakers

@iamareebjamal Now I have to work on when going from different page to diff page section.
but being on same page its working?

@iamareebjamal
Copy link
Member

Yes

1 similar comment
@iamareebjamal
Copy link
Member

Yes

@divyamtayal
Copy link
Member Author

@iamareebjamal made the changes. Now getting to section from diff page is working properly.

@lgtm-com
Copy link

lgtm-com bot commented Nov 6, 2020

This pull request introduces 1 alert when merging 3fa86e5 into 79e0cbd - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@divyamtayal
Copy link
Member Author

divyamtayal commented Nov 6, 2020

@iamareebjamal I mean i got the way how it will be working and its working what was said but the problem that will be arising here is that I have used setTimeout funtion for 2 sec but this won't work always bcz what if page took more than 2 sec so I want to know is there a way so that function gets executed only when the page is completely loaded?
or todo something after the transition is done completely?

@lgtm-com
Copy link

lgtm-com bot commented Nov 6, 2020

This pull request introduces 1 alert when merging 9bc5959 into 79e0cbd - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@divyamtayal
Copy link
Member Author

divyamtayal commented Nov 6, 2020

@iamareebjamal https://open-event-frontend-mq4nd06op.vercel.app/e/1f720482/speakers
and clicking on sponsors is working and even other buttons are working

@iamareebjamal
Copy link
Member

Fix the build and indentation. Cannot review till then

@lgtm-com
Copy link

lgtm-com bot commented Nov 7, 2020

This pull request introduces 1 alert when merging a35c4a5 into 4fa8591 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@divyamtayal
Copy link
Member Author

divyamtayal commented Nov 20, 2020

@iamareebjamal I have Improved the code for TODO-1 and it looks more cleaner then before.
looking for better solution for TODO-2.
Did u get any of it other then setTimeout?

@divyamtayal
Copy link
Member Author

@iamareebjamal I was reading about ember run loops which I think might help in this case

@iamareebjamal
Copy link
Member

They help when you want to run some code in next event loop iteration

@divyamtayal
Copy link
Member Author

divyamtayal commented Nov 22, 2020

@iamareebjamal Is there a way in which I could run a code after the DOM is completely rendered?
didElementInsert only happens first time, I want everytime.

@iamareebjamal
Copy link
Member

What's the usecase? MutationObserver may help but there might be better solution for your specific usecase

@iamareebjamal
Copy link
Member

After latest changes, I can't go from other page to info link. Go to speakers and then click on sponsors. Not working

@divyamtayal
Copy link
Member Author

What's the usecase? MutationObserver may help but there might be better solution for your specific usecase

I was thinking of the way in which we can get previous visted route and if the route is speaker or coc etc. we can do dom manipulation like here scrolling.
Now the problem is I can get previous route but need to do DOM after render is complete.

@divyamtayal
Copy link
Member Author

After latest changes, I can't go from other page to info link. Go to speakers and then click on sponsors. Not working

Its working for me

@iamareebjamal
Copy link
Member

iamareebjamal commented Nov 22, 2020

Easier thing would be to abandon LinkTo or href-to helper and just use pure anchor tags and see if things work

@divyamtayal
Copy link
Member Author

Easier think would be to abandon LinkTo or href-to helper and just use pure anchor tags and see if things work

no it does not work

@divyamtayal
Copy link
Member Author

@iamareebjamal after lot of things doing and researching for this solution, I found a workaround which will be working very good for TODO-2

@divyamtayal
Copy link
Member Author

@iamareebjamal I have implemented this method for going to section from diff page.
Pls let me know to do any changes.

@iamareebjamal
Copy link
Member

It's not going to the section from different page

https://open-event-frontend-git-double-click-5123.eventyay.vercel.app/e/3dbaaa50/coc

Now click on sponsors or organizer

@iamareebjamal
Copy link
Member

There's no need for goTo action. This could be simplified even more and thus I have done that. Please check if everything is working as expected

@iamareebjamal iamareebjamal changed the title fix: To get to sections on the same page that are listed on the menu requires two clicks fix: Handle side menu navigation correctly Nov 24, 2020
@iamareebjamal iamareebjamal merged commit c74edec into fossasia:development Nov 24, 2020
@divyamtayal
Copy link
Member Author

There's no need for goTo action. This could be simplified even more and thus I have done that. Please check if everything is working as expected

Yep everything is working fine. Thanks for improving it.

@iamareebjamal
Copy link
Member

Great. Please check gitter

@divyamtayal divyamtayal deleted the double-click-5123 branch November 27, 2020 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Public Event Page Menu: Moving to Sponsors, Organizers etc. requires two clicks
2 participants