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

fixed footer issue #76

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

sauravrajleaf
Copy link

Issue

Fixes #75

Description

New content container add to Home & Bookmarked. Modified footer css.

Type of change

  • Bug fix Css styling issue of footer.  #75 (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation change

Screenshot

Screenshot_Mozilla Firefox_50
Screenshot_Mozilla Firefox_51
Screenshot_Mozilla Firefox_52
Screenshot_Mozilla Firefox_53

Checklist:

  • My code follows the style guidelines of this project
  • The app runs properly
  • I have performed a self-review of my own code
  • I have formatted and commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@sauravrajleaf
Copy link
Author

Please check if this can be merged.

Copy link
Owner

@Sparsh1212 Sparsh1212 left a comment

Choose a reason for hiding this comment

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

@sauravrajleaf Please remove all the editor formatting changes like double quotes, semicolons, etc.
A quicker way to do them would be to revert all the current changes and turn auto-format on save off. Now do the correct changes in CSS and then push them to your PR.
After this you are free to turn it on again :)

@Sparsh1212
Copy link
Owner

Please remove the merge conflicts too.

@sauravrajleaf
Copy link
Author

  • As per the instructions, I have reverted all the changes done in all the files which was affected due to the code editor format on save. Later, I made changes to the required CSS.
  • Also regarding the merge conflict it was occurring in Home.js file which was also changed by the editor on save option as per my knowledge. So, in my opinion reverting everything would have solved that too.

@sauravrajleaf
Copy link
Author

Regarding the last part pushing them to my PR. I did not understand that very well.

  • Should I make a new PR & close this one?
  • Should I commit the changes to same PR?

If it's the later one, I have no idea to how to push changes to same PR. Can you please guide me through. Thanks.

@Sparsh1212
Copy link
Owner

Sparsh1212 commented Jan 2, 2022

  • As per the instructions, I have reverted all the changes done in all the files which was affected due to the code editor format on save. Later, I made changes to the required CSS.
  • Also regarding the merge conflict it was occurring in Home.js file which was also changed by the editor on save option as per my knowledge. So, in my opinion reverting everything would have solved that too.

Just a correction to your second point: The merge conflict is not due to the editor. It occurs when someone also proposed some changes on the same line of code that you were working and his/her changes got merged before you. That's why you faced a merged conflict. To resolve this, I'd suggest you to watch any tutorial on "How to resolve merge conflicts"

@Sparsh1212
Copy link
Owner

Regarding the last part pushing them to my PR. I did not understand that very well.

  • Should I make a new PR & close this one?
  • Should I commit the changes to same PR?

If it's the later one, I have no idea to how to push changes to same PR. Can you please guide me through. Thanks.

By pushing I meant, just do a new commit to your current PR and push those latest changes. By pushing, your newly added commits will also reflect here, and I will be able to see your updated code.

@sauravrajleaf
Copy link
Author

I've done the changes. Please check once. I hope this time it's right.

Copy link
Owner

@Sparsh1212 Sparsh1212 left a comment

Choose a reason for hiding this comment

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

The web view is working totally fine. However, in the mobile view the UI is breaking.
Here take a look:
Screenshot 2022-01-03 at 12 25 44 PM
Please make sure it is fine in the mobile view too.

Also please revert the changes made to package-lock.json as those also have been made automatically by your ide.

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.

Css styling issue of footer.
2 participants