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

ITSNP-3: Design Card components #11

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

ITSNP-3: Design Card components #11

wants to merge 10 commits into from

Conversation

bijudhungel11
Copy link

  • Designed posts trending component card
  • created resources folder under src which contains images
  • created constants folder for importing all images from resources folder
  • made mixin for font and display item in flex
  • shame: design is not responsive

- Designed posts trending component card
- created resources folder under src which contains images
- created constants folder for importing all images from resources folder
- made mixin for font and display item in flex
- shame: design is not responsive
@SurrealSerenade
Copy link
Collaborator

SurrealSerenade commented Jun 9, 2021

@bijudhungel11 Please link the issue related to this pull request (PR). Also, name the PR as ITSNP-03: Design Card Components. Thank you.

@RudeSoul
Copy link
Collaborator

RudeSoul commented Jun 9, 2021

I think this is the first thing you need to fix,

image

Copy link
Collaborator

@SurrealSerenade SurrealSerenade left a comment

Choose a reason for hiding this comment

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

Please, have a look at the comments. Thank you.

public/index.html Outdated Show resolved Hide resolved
src/components/PostsTrendingCard/PostsTrendingCard.js Outdated Show resolved Hide resolved
src/components/PostsTrendingCard/PostsTrendingCard.js Outdated Show resolved Hide resolved
src/components/PostsTrendingCard/PostsTrendingCard.scss Outdated Show resolved Hide resolved
src/components/PostsTrendingCard/PostsTrendingCard.scss Outdated Show resolved Hide resolved
src/components/PostsTrendingCard/PostsTrendingCard.scss Outdated Show resolved Hide resolved
src/constants/Image.js Outdated Show resolved Hide resolved
src/styles/utils/_mixins.scss Outdated Show resolved Hide resolved
Copy link
Collaborator

@RudeSoul RudeSoul left a comment

Choose a reason for hiding this comment

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

Nice pr, just some issues, Hope you will resolve asap

public/index.html Outdated Show resolved Hide resolved
src/components/PostsTrendingCard/PostsTrendingCard.js Outdated Show resolved Hide resolved
src/pages/Home/Home.js Show resolved Hide resolved
- removed font family link tag from index.html
- provided data through  props in posts Trending card component
- added imported react from library to the top of component
- used naming convnetion to capitalize the allImage to AllImage
- created icons folder to keep the all svg icons as component
- used BEM mixins in post trednindg card componenet scss file
- tried to created reusable mixin for fonts
@SurrealSerenade SurrealSerenade changed the title Design Card components ITSNP-11: Design Card components Jun 10, 2021
@SurrealSerenade SurrealSerenade changed the title ITSNP-11: Design Card components ITSNP-3: Design Card components Jun 10, 2021
@bijudhungel11 bijudhungel11 linked an issue Jun 10, 2021 that may be closed by this pull request
5 tasks
@bijudhungel11 bijudhungel11 marked this pull request as draft June 10, 2021 09:34
@SurrealSerenade SurrealSerenade added design Design request good first issue Good for newcomers new component For creating new component labels Jun 10, 2021
@SurrealSerenade SurrealSerenade added this to In progress in ITSNP Blog via automation Jun 10, 2021
bijudhungel11 and others added 3 commits June 11, 2021 14:33
- changed font-weight-family mixin to font
- try to make reusable component
- removed flex-item-center
- have completed BlogPostCard
- have completed SmalCardBlogPost Card and done it's style as welll
- have comleted MediumCardBlogPost Card
- imported all images from resources folder
- shame: Overall Design is not completed and any other mixins has not been made
- shame: haven't work deeply need to fix lot's of stuff
- shame: haven't exported image directly from Image.js as we are guided to export it directly from Image.js
- changed image imoprt to svg component import
- tried to provide the props for all dyanamic data
- changed some scss to make pixel perfect
- shame: still some scss is not refactored
@bijudhungel11 bijudhungel11 moved this from In progress to Review in progress in ITSNP Blog Jun 19, 2021
@bijudhungel11 bijudhungel11 marked this pull request as ready for review June 19, 2021 10:46
reganspk and others added 2 commits June 19, 2021 18:48
- resolve Images
- Removed constants folder to import all images
- directly importing from the resources folder to the component
src/pages/Home/Home.js Show resolved Hide resolved
src/components/BlogPostCard/index.js Outdated Show resolved Hide resolved
src/components/Circles/LargePink.js Outdated Show resolved Hide resolved
- changed caption naming convention for alternative attribut in image tag with imageTitle props
- tried to use Avatar component which was call from  profileComp branch made by ace4port
- moved the circles folder to joinMadhyam component folder where it belongs
Copy link
Collaborator

@SurrealSerenade SurrealSerenade left a comment

Choose a reason for hiding this comment

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

Exceptional Work, Keep it up! Do fix the minor comments though.

imageTitle,
authorImg,
date = "May 17",
authorName = "XettriAl.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does not make much sense to have default values for props date and authorName as "May 17" and "XettriAl.".

import SmallPink from "./Circles/SmallPink";
import Purple from "./Circles/Purple";
const JoinMadhyamCard = ({
topic = "Join Mnadhyam!",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please correct this spelling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design request good first issue Good for newcomers new component For creating new component
Projects
ITSNP Blog
Review in progress
Development

Successfully merging this pull request may close these issues.

Design Card components (m1)
5 participants