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

Fixes #77:Hidden top on upscrolling #142

Merged
merged 1 commit into from
Apr 26, 2017
Merged

Fixes #77:Hidden top on upscrolling #142

merged 1 commit into from
Apr 26, 2017

Conversation

KukkadapuSushma
Copy link
Contributor

@KukkadapuSushma KukkadapuSushma commented Apr 18, 2017

Here are few screenshots to visualize the change 👍
Link for demo : https://sushmakukkadapu.github.io/
@daminisatya @jig08 @mariobehling

image

.

image

.

image

@mariobehling
Copy link
Member

Please check failing tests and provide a test site on your GitHub pages.

@mariobehling mariobehling requested a review from djmgit April 18, 2017 16:46
@KukkadapuSushma
Copy link
Contributor Author

Test site link : https://sushmakukkadapu.github.io/ @mariobehling

@mariobehling
Copy link
Member

Thank you.

The move up of the top bar is delayed. I am not sure why. Please implement it without any delay.

@KukkadapuSushma
Copy link
Contributor Author

Yeah sure, @mariobehling will set the delay thing, but before I create a new PR, does this existing PR needs to be merged or cleared? (in order to avoid clash)

@mariobehling
Copy link
Member

You need to update your branch.

Copy link
Member

@djmgit djmgit left a comment

Choose a reason for hiding this comment

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

@KukkadapuSushma Please remove the trailing white spaces so that travis doesn't show failures.
You can remove the console statements from the function you have added in loklak.js as I guess those statements were used for debugging purpose.
One more thing, On scrolling up, the search bar (top bar) gets hidden (after some delay as Mario has already mentioned) but when I scroll back down and the top bar reappears, the gray bar and the green bar beside it are not vertically aligned. Could you please fix it.Also make sure that the entire thing is also working in mobile view, as of now in mobile view when you scroll up the top bar is not entirely getting hidden.

@KukkadapuSushma
Copy link
Contributor Author

KukkadapuSushma commented Apr 22, 2017

@djmgit @jig08 @mariobehling
Thanks for the review, I have made changes! Have a look

Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

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

The delay when scrolling up is still there.

@KukkadapuSushma
Copy link
Contributor Author

@mariobehling Ok sure I will see through it again.

@KukkadapuSushma
Copy link
Contributor Author

https://sushmakukkadapu.github.io/
@mariobehling Kindly review!

@mariobehling
Copy link
Member

Please compare to https://play.google.com and check how you can speed up the feature. Thanks

@mariobehling
Copy link
Member

Ok, let's speed this up. I am merging this and we can iterate from here. Thank you.

@mariobehling mariobehling merged commit 5d0abd4 into loklak:master Apr 26, 2017
@KukkadapuSushma
Copy link
Contributor Author

Thanks for the merge, will speed up, I have understood after looking thoroughly at https://play.google.com .

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

Successfully merging this pull request may close these issues.

3 participants