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

Added a section of home page #134

Closed
wants to merge 19 commits into from
Closed

Conversation

raksh543
Copy link

@raksh543 raksh543 commented Aug 28, 2020

Description

Added the home section of Getting started, ways to contribute, our projects and open source programs

Fixes #110

Type of Change:

Delete irrelevant options.

  • Code
  • User Interface

Code/Quality Assurance Only

  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

I have checked the responsiveness by resizing window.

Landscape view:
b

Potrait view:
a

Checklist:

Delete irrelevant options.

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas

Code/Quality Assurance Only

  • My changes generate no new warnings

@annabauza
Copy link
Contributor

@raksh543 please separate this to 4 sections and each section should follow our implementation guide. Thanks.

@annabauza annabauza closed this Aug 29, 2020
@annabauza annabauza reopened this Aug 29, 2020
Copy link
Contributor

@annabauza annabauza left a comment

Choose a reason for hiding this comment

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

@raksh543 please separate this to 4 sections and each section should follow our implementation guide. Thanks.

@annabauza annabauza added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: Changes Requested Changes are required to be done by the PR author. labels Aug 29, 2020
@annabauza
Copy link
Contributor

@raksh543 I had something bit more reusable in my mind. Can you please review this PR There is ImageTextSection which you could just use? Thanks in advance. I think I will need to add paragraphs to keep consistency.

@annabauza
Copy link
Contributor

@raksh543 I have applied the changes, can you please review?

Copy link
Author

@raksh543 raksh543 left a comment

Choose a reason for hiding this comment

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

@annabauza I have reviewed the changes.
It looks good.

@annabauza
Copy link
Contributor

@raksh543 the PR has been merged so you can reuse TextImage component. Please prepare testing page by following the guide https://github.com/anitab-org/anitab-org.github.io/wiki/Prepare-testing-GitHub-Pages-for-your-PR

@raksh543
Copy link
Author

raksh543 commented Sep 6, 2020

@annabauza Please check the preview of the webpage, if it looks fine now i will proceed.

@raksh543
Copy link
Author

raksh543 commented Sep 8, 2020

@annabauza Please review the changes

Copy link
Contributor

@annabauza annabauza left a comment

Choose a reason for hiding this comment

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

  1. No need to add other fonts,
  2. image requests should come from content - look on how other code parts are solving it - replace json with js file
  3. page should not display on each tab - only when you press on AnitaB logo (if case 0 in Content.js)
  4. use styled view like aboutus/styles.js
  5. use mapper instead of create each section file separately.
  6. remove dependency which you have added.

const {sections} = contentJson;
var content ="";

sections[0].content.forEach(text=>{
Copy link
Contributor

Choose a reason for hiding this comment

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

please pass section as a prop

var content ="";

sections[0].content.forEach(text=>{
content += text.par+"\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

keep paragraphs as stack of texts instead of adding '\n'

@raksh543
Copy link
Author

@annabauza I am working on these changes.
Will update soon.

@raksh543
Copy link
Author

@annabauza I have made all changes as requested. Please review.

Copy link
Contributor

@annabauza annabauza left a comment

Choose a reason for hiding this comment

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

@raksh543 thank you for this PR, I have added those sections can you please update the images on home page? Thanks!

@nandini45
Copy link
Member

@raksh543 can u do the changes??
Do update us asap if not I have to assign this issue to someone else.

@raksh543
Copy link
Author

@nandini45 Sure i will do the changes. Please tell me which which images i am supposed to change.

@Vishal-raj-1
Copy link

@anna4j Any update on this PR? I can see this PR contains many conflicting files. Please resolve conflicts and you can squash your 19 commits to single commit. Thanks !!

@annabauza
Copy link
Contributor

closing this issue because getting started already exists.

@annabauza annabauza closed this Jan 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coding: Develop the following section for Home Page
4 participants