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 #163 - Added footer #164

Closed
wants to merge 1 commit into from
Closed

fixes #163 - Added footer #164

wants to merge 1 commit into from

Conversation

djmgit
Copy link
Member

@djmgit djmgit commented May 29, 2017

Short description

Fixes issue #163. The main page now contains a footer.

I have:

  • There is a corresponding issue for this pull request.
  • Mentioned the Issue number in the pull request commit message Fixes #<number> commit message
  • There is only strictly only one commit per issue.

For the reviewers

I have:

  • Reviewed this pull request by an authorized contributor.
  • The reviewer is assigned to the pull request.

@djmgit
Copy link
Member Author

djmgit commented May 29, 2017

test link: http://loklakapps.surge.sh/

@SKrPl @kavithaenair open for review

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.

  1. The "create loklak app" link simply goes to Github. I would expect a tutorial at this link. We don't have that now, therefore, please take this out.
  2. The bottom bar should be same as on loklak.net. Please implement the same bar and ensure the alignments are correct. Right now the alignment of the right side menu is not matching.

@djmgit djmgit force-pushed the footer branch 3 times, most recently from 2dbf26c to 3671209 Compare May 30, 2017 13:08
@mariobehling
Copy link
Member

Footer no longer visible on test page.

@kavithaenair
Copy link
Member

kavithaenair commented Jun 2, 2017

As @mariobehling said, the test link is not showing the footer anymore. @djmgit please look into it.

@djmgit
Copy link
Member Author

djmgit commented Jun 2, 2017

@kavithaenair I have removed it, due to some glitches, I am working on it, will update the PR ASAP

@djmgit
Copy link
Member Author

djmgit commented Jun 19, 2017

@kavithaenair @Achint08 @SKrPl updated my PR

Copy link
Contributor

@hemantjadon hemantjadon left a comment

Choose a reason for hiding this comment

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

@djmgit As you can see that the footer starts from the middle of the screen and moves as the content loads. This is not at all ideal, Make sure the footer starts at the bottom. I don't know how the page is getting the data items, and why there is content adjustment dynamically, this is not at all pleasing, we can have a separate issue for this, but for now, let's make the footer start at bottom and not at the middle of the screen

@djmgit
Copy link
Member Author

djmgit commented Jun 19, 2017

@hemantjadon sure, I will fix it.

Fixes issue loklak#163. The main page now contains a footer and
store listing page now contains a footer.
@djmgit
Copy link
Member Author

djmgit commented Jun 20, 2017

@hemantjadon updated PR

@singhpratyush
Copy link
Member

@djmgit: The changes look good. Please resolve conflicts and rebase.

@hemantjadon
Copy link
Contributor

@djmgit Is the link also updated? I dont think it is, It is still showing moving footer.
screen shot 2017-06-23 at 5 49 00 pm

@djmgit
Copy link
Member Author

djmgit commented Jun 23, 2017

@hemantjadon strange!! I have updated the link, I tested it on chrome and firefox, the footer is not appearing in the middle, even if it appears (is some one scrolls the page while loading), it appears right at the bottom. Which browser are you using? Could you please also test the details page?

@SKrPl
Copy link

SKrPl commented Jun 23, 2017

@djmgit There are some merge conflicts, and I also can't see footer in both homepage and app's details page.

Copy link
Member

@kavithaenair kavithaenair left a comment

Choose a reason for hiding this comment

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

LGTM 👍 . Please resolve conflicts.

@hemantjadon
Copy link
Contributor

@djmgit Am I checking the wrong link? http://loklakapps.surge.sh/
The footer is still starting in the mid.

@djmgit
Copy link
Member Author

djmgit commented Jul 3, 2017

@hemantjadon yeah, that is the current link, which browser are you using??

@hemantjadon
Copy link
Contributor

@djmgit I am using Chrome 59

@Achint08
Copy link

Achint08 commented Jul 4, 2017

@djmgit It is blinking for me. OSX Firefox.
Checkout this out
https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Scroll-linked_effects

@djmgit
Copy link
Member Author

djmgit commented Jul 4, 2017

Okay, I guess this is due to compatibility issues across various browsers. Closing it for now, will be working on it later.

@djmgit djmgit closed this Jul 4, 2017
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.

7 participants