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

Create Single Blog Post UI #215

Merged
merged 18 commits into from
Jun 14, 2024
Merged

Create Single Blog Post UI #215

merged 18 commits into from
Jun 14, 2024

Conversation

Satoshi-Sh
Copy link
Member

@Satoshi-Sh Satoshi-Sh commented May 30, 2024

Web Dev Path
214

Have you updated the CHANGELOG.md file? If not, please do it.

Yes, I did.

What is this change?

Made a single blog post page according to the Figma file created by @kimbercash .

Were there any complications while making this change?

  • I used this styling style for the first time, and needed to learn how the structure is organized.

If necessary, please describe how to test the new feature or fix.

Please go to /blog/post/1. You need to type it in the url. You can see the page with my sample blog post from dev.to.

Comments

  • I believe the image is the cover picture in the Figma. I cannot insert it between the blog script. I just put it under the title. The post I used as a sample doesn't have images inside the script. I need to see what it looks like and modify the code accordingly.
  • I tried to match the layout to the Figma file as much as I could, but feel free to ask to make it more precise.
  • Is it possible to change the SVG logo colors? I just put black ones for now.

Screenshot

blog-post-demo

When should this be merged?

After dev.to API is incorporated.

@Satoshi-Sh Satoshi-Sh self-assigned this May 30, 2024
@Satoshi-Sh Satoshi-Sh requested a review from cherylli May 30, 2024 21:38
Copy link

netlify bot commented May 30, 2024

Deploy Preview for webdevpathstage ready!

Name Link
🔨 Latest commit 5de84d0
🔍 Latest deploy log https://app.netlify.com/sites/webdevpathstage/deploys/66639e850c9c2000082e3d53
😎 Deploy Preview https://deploy-preview-215--webdevpathstage.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Satoshi-Sh Satoshi-Sh requested review from a team and mariana-caldas May 30, 2024 21:38
@cherylli
Copy link
Member

It looks good, but it seems the PR is not ready, if that's the case, you can change this to a draft PR

I understand /blog/post/1 is temporary, so I'm not sure what your plan is but I think we should use the slug instead of the Id because it looks nicer and better for SEO

Is it possible to change the SVG logo colors? I just put black ones for now.

I believe you can change the the color in the SVG code

@Satoshi-Sh Satoshi-Sh marked this pull request as draft May 31, 2024 03:01
@Satoshi-Sh
Copy link
Member Author

Thanks for checking Cheryl.

I switched it to draft. This task is for the UI part. I believe someone else will take over this branch once my part is done.

I will play with the SVG color to match the Figma file.

@mariana-caldas
Copy link
Member

Hey, @Satoshi-Sh ! That's a great start, thanks!

Here are a few points we need to work on here:

  1. A DRAFT PR means a work in progress without reviewers. You must remove your reviewers if you're still working on the PR. They will be added again as soon as the work is ready for review.

  2. To keep the styling consistent, let's use all the styled-components we already have instead of creating new ones: https://github.com/Web-Dev-Path/web-dev-path/tree/main/components/containers/Container. The same applies to headers and paragraphs we get from the theme itself.

  3. Please take a look at our utils folder to get an idea of how we can organize content information. We'll get that content dynamically later, as Cheryl suggested in the next PR.

Please let me know if that makes sense!

@Satoshi-Sh Satoshi-Sh removed request for a team, cherylli and mariana-caldas June 2, 2024 22:45
@Satoshi-Sh
Copy link
Member Author

Thanks for checking @mariana-caldas . We don't want to merge this branch into the main until we implement the dev to API part. Should I keep this as a draft even though I'm done?

To keep the styling consistent, let's use all the styled-components we already have instead of creating new ones: https://github.com/Web-Dev-Path/web-dev-path/tree/main/components/containers/Container. The same applies to headers and paragraphs we get from the theme itself.

I replaced S.Container with Container except for the AuthorBio container and S.Title with Title. Does this look fine? About other headings and paragraphs, do we have any other components I can use here?

Please take a look at our utils folder to get an idea of how we can organize content information. We'll get that content dynamically later, as Cheryl suggested in the next PR.

I just moved the sample data to the util folder for better visibility.

@mariana-caldas
Copy link
Member

Thanks for checking @mariana-caldas . We don't want to merge this branch into the main until we implement the dev to API part. Should I keep this as a draft even though I'm done?

To keep the styling consistent, let's use all the styled-components we already have instead of creating new ones: https://github.com/Web-Dev-Path/web-dev-path/tree/main/components/containers/Container. The same applies to headers and paragraphs we get from the theme itself.

I replaced S.Container with Container except for the AuthorBio container and S.Title with Title. Does this look fine? About other headings and paragraphs, do we have any other components I can use here?

Please take a look at our utils folder to get an idea of how we can organize content information. We'll get that content dynamically later, as Cheryl suggested in the next PR.

I just moved the sample data to the util folder for better visibility.

This is looking so much better, @Satoshi-Sh ! I think we're almost there with this UI; great job so far!

Two points to have in mind:

Screenshot 2024-06-06 at 2 18 59 PM

  1. Please make sure the author bio section is also using the same container element so we keep everything consistent:

  2. About the headings' styling, can we just import what we have from the https://github.com/Web-Dev-Path/web-dev-path/blob/main/styles/themeConfig.js from line 90 by only adding the simple elements p, h3, and any other element directly to the component markup ?

Your PR gave me a cool idea for a blog post btw 🙏🏼 . Getting started with a new codebase to implement features. Any thoughts @cherylli ?

@Satoshi-Sh
Copy link
Member Author

Satoshi-Sh commented Jun 7, 2024

I replaced S.Container with Container except for the AuthorBio container

I forgot to mention about it. I need to override the default style of the Container. That's why I kept the new Container. In the new commit, I use an overridden Container. Does it look fine?

About the headings' styling, can we just import what we have from the https://github.com/Web-Dev-Path/web-dev-path/blob/main/styles/themeConfig.js from line 90 by only adding the simple elements p, h3, and any other element directly to the component markup ?

I'm not sure about this instruction. It would be great if you could give me an example.

P.S. Do you mean I just need to use the default (defined in GlobalStyles) <p> <h3> instead of making a new Title,Subtitle, and so on?

@Satoshi-Sh Satoshi-Sh marked this pull request as ready for review June 8, 2024 00:01
@Satoshi-Sh Satoshi-Sh requested review from cherylli, mariana-caldas and a team and removed request for cherylli and mariana-caldas June 8, 2024 00:02
Copy link
Member

@cherylli cherylli left a comment

Choose a reason for hiding this comment

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

Looks good, everything works fine

@mariana-caldas mariana-caldas requested review from mariana-caldas and a team June 11, 2024 00:10
Copy link
Member

@mariana-caldas mariana-caldas left a comment

Choose a reason for hiding this comment

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

Great job, @Satoshi-Sh ! 🥇

Copy link
Member

@LuSilvaDeveloper LuSilvaDeveloper left a comment

Choose a reason for hiding this comment

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

Everything looks good and it's working properly. Great job @Satoshi-Sh

@Satoshi-Sh Satoshi-Sh merged commit 81f142d into main Jun 14, 2024
4 checks passed
@Satoshi-Sh Satoshi-Sh deleted the feat/single-post-ui branch June 14, 2024 01:30
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.

4 participants