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

Public Schedule: Clicking on a filter moves the page #5826 #5849

Closed
wants to merge 4 commits into from

Conversation

akash2408
Copy link
Contributor

@akash2408 akash2408 commented Dec 1, 2020

Fixes #5826

Short description of what this resolves:

adding ember install ember-router-scroll

Changes proposed in this pull request:

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 Dec 1, 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/8t6059who
✅ Preview: https://open-event-frontend-git-development.eventyay.vercel.app

Copy link
Contributor

@sachinchauhan2889 sachinchauhan2889 left a comment

Choose a reason for hiding this comment

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

issue is still there. page is still moving after filtering sessions.

@lgtm-com
Copy link

lgtm-com bot commented Dec 1, 2020

This pull request introduces 1 alert when merging ef35cdd into 3a1e29c - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

Copy link
Contributor

@sachinchauhan2889 sachinchauhan2889 left a comment

Choose a reason for hiding this comment

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

Try this...
just remove sessionChanged() action----> line 21-29.
and also remove 'sessionChanged' from this.addobserver()----> in line 18.

according to me this resolve issue.

scrollTop: $('#session-heading').offset()?.top
});
}
this.set('preserveScrollPosition', true);
Copy link
Contributor

Choose a reason for hiding this comment

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

why created this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After only removing session changed whenever we click on filter page will reload and moves up using previous scoll position this well save the earlier scroll postion of page and didnt moves the page up and down

Copy link
Contributor

Choose a reason for hiding this comment

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

you are right. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

good work @akash2408

@sachinchauhan2889
Copy link
Contributor

sachinchauhan2889 commented Dec 1, 2020

@iamareebjamal sir, looks good to me.

please review

@codecov
Copy link

codecov bot commented Dec 1, 2020

Codecov Report

Merging #5849 (85e8bb0) into development (3a1e29c) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #5849   +/-   ##
============================================
  Coverage        23.46%   23.46%           
============================================
  Files              512      512           
  Lines             5460     5459    -1     
  Branches            63       63           
============================================
  Hits              1281     1281           
+ Misses            4163     4162    -1     
  Partials            16       16           
Impacted Files Coverage Δ
app/controllers/public/sessions.js 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a1e29c...85e8bb0. Read the comment docs.

@mariobehling
Copy link
Member

mariobehling commented Dec 1, 2020

It is not working. The page should not move at all. Date, track and rooms still result in moving the page. The page should not move at all where-ever it is. It currently moves to the heading session.

@akash2408
Copy link
Contributor Author

It is not working. The page should not move at all. Date, track and rooms still result in moving the page. The page should not move at all where-ever it is. It currently moves to the heading session.

Sir, Thats why i remove the content of sesssion changed function.The gif was of previous commit .please review again

@iamareebjamal
Copy link
Member

@akash2408 That's because you pushed from development branch. Never ever create a PR from development branch. Now, we can't preview your work

@iamareebjamal
Copy link
Member

Superseded by #5878

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Public Schedule: Clicking on a filter moves the page
4 participants