Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Express Blog #1519

Merged
merged 47 commits into from
Jul 22, 2024
Merged

Express Blog #1519

merged 47 commits into from
Jul 22, 2024

Conversation

chrisdel101
Copy link
Contributor

@chrisdel101 chrisdel101 commented May 18, 2024

So hoping you can live preview this before making it live. With that in mind, I've not added any photos.

The feature includes the /posts page, the /write-posts page, and the /example-post page. The last two are very similar. If they are too similiar we can get rid of the latter.
You can visit the individual posts themselves by clicking on the title
Review item: Would you rather have the entire div clickable? Or okay as is?

There are some changes made to the search bar to make it fit with added list items. It's only been tested on mac, so hopefully it works on windows.

The gutter menu is just another listing of the blog links. There is also a menu for tags, but since there are not enough tags, this isn't super useful right now. If it's okay to just leave this unused, great. Else, we can delete it from the code base.

Last, there are 3 dummy posts used to populate the posts page. After preview reviewing is complete, we will need to delete those as they are only for the purposes of review. Those are the ones entitled sample-post.

If there are no posts this is what displays
Screenshot 2024-05-18 at 3 07 40 PM

Let me know if you find any bugs :)
PS - I have no idea what those build errors mean.
PSS - I added some comments to the files so you know what they are doing (I seriously hope you did not get notifications for all of those! If you did, sorry!) Also, I assigned you guys to this PR. I hope that's okay :)

- add blog post to header
- add min height to posts page
- add blog/posts index page
- add single template style to posts page; will handle all post displays
- build posts template styles
- add futher posts index styles (based on shopify blog UI)
- remove parens in liquid
- liquid func to strip out top part of excerpt
- add active to first item
- add gutter links for tags
- add JS to handle gutter tags
- add CSS for gutter + posts
- add blog specfic layout
- add search bar style adjustments
- adjust tags side menu and remove from workflow
- add in second side menu
- add How To write Post page
- add more styles
- add example post page layout
- add all links to side menu
- readd post width 100%; imgs are not all same size this way
- add border to post
- change menu text color
- save 3 sample posts to use for preview - will need deletig
- update file names
- change file name
Copy link

netlify bot commented May 18, 2024

Deploy Preview for expressjscom-preview ready!

Name Link
🔨 Latest commit bfa2fc7
🔍 Latest deploy log https://app.netlify.com/sites/expressjscom-preview/deploys/669725d372aaad0008bc2203
😎 Deploy Preview https://deploy-preview-1519--expressjscom-preview.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.

@chrisdel101 chrisdel101 changed the title Blogging Features: PR for preview only Blogging Features: PR first for preview May 18, 2024
@chrisdel101 chrisdel101 self-assigned this May 18, 2024
- change updated file name
- fix wrong example link
- fix wrong div placement
_layouts/blog-posts.html Outdated Show resolved Hide resolved
en/blog/example-post.md Outdated Show resolved Hide resolved
@carlosstenzel
Copy link
Contributor

carlosstenzel commented Jun 20, 2024

I tried to help, but I'm not allowed to resolve conflicts :(

Captura de Tela 2024-06-20 às 18 36 17

I can create a view-only PR if that helps

#1529

@chrisdel101
Copy link
Contributor Author

Good idea using the layout to solve the problem. I moved all the logic there expect the basic stuff.
I updated the writing guide too.
The CSS on the hover state could be a bit nicer, so I can try work on that. unless you're good at nice hover UIs.

- lower transition fade time
@chrisdel101
Copy link
Contributor Author

lowered transition time and I think it looks better.

@crandmck
Copy link
Member

Just a note to say that I want to work with this, and I have a few final changes in mind, but I've been down and out with Covid for the last week.
This PR is getting close to being good to go.

@crandmck
Copy link
Member

crandmck commented Jul 3, 2024

I made a bunch more changes to the fork/branch:

  • Changed the blog menu to link to the most recent post, not list of all posts (which is no longer used, but I left blog/posts.md there for now. There might be a better way to do this, but for now it should suffice. For several reasons, I think the main blog page should show only the most recent post; for example SEO. The sidebar posts menu provides access to all the other posts.
  • Made some general cleanup edits to "How to write a blog post" and added a small illustration (generated with Adobe Firefly).
  • For now, let's assume blog posts won't be translated. I think I removed all the lang refs, but need to check for consistency.

Currently, there's an error in the terminal when the image is loaded

ERROR `/images/blogger@2x.jpg' not found.

I assume this is something to do with media queries. Not sure if it's anything we need to fix or if we can just ignore it.

At this point, I think this PR is "done enough" to prepare it for final review and merging:

  1. Remove the "dummy" posts from _posts
  2. Change the date in _posts/2024-05-25-welcome-post.md and perhaps change the file name as well.
  3. Any other final edits?

@chrisdel101
Copy link
Contributor Author

chrisdel101 commented Jul 4, 2024

These are some significant changes.
By removing the conditional statement from the post.html the subtitle shows up on the main page. I don't think we want this.
Screenshot 2024-07-03 at 9 29 35 PM

But if your wanting to get rid of the blog posts main listing page entirely, /posts, then the above issue doesn't matter. Otherwise we need that conditional, or another piece of logic.

For me, a link called posts in the nav, linking to a single post, is misleading, while the side menu link also called posts links to a lists of all posts? Posts should have one meaning.
Perhaps removing the dropdown would be the way to go? Then only access would be to the newest post from there by clicking blog. And access all others via the side menu - however, this side menu is not visible in mobile - so what then?
Also we'd need to add the "write a post" link on each post individually, via the template.

I personally think we should either

  • link directly to the all posts from the nav bar, like it is was.
  • Or remove the all post page entirely and have the menu show up on each individual post. (Again, how would we navigate on mobile?)
    Both are some extra work to achieve. But I don't see the point of an all posts page that is not the main entry point to the blog.

We could also keep the all posts page and change the name in the nav bar to like Current Post. But still not sure how we'd navigate to any pages on mobile.

Good idea to add lang back in. This was missing.

Let me know how you want to do it.

Also the nav bar link appears to be broken at the moment in preview.

@chrisdel101
Copy link
Contributor Author

OR:

  • we could list all the posts in the dropdown (and get rid of the all posts page). This would solve navigation.

@crandmck
Copy link
Member

crandmck commented Jul 5, 2024

Sorry for the scope of the changes @chrisdel101; I just thought it would be easier to demonstrate what I had in mind rather than explain and discuss it. Essentially, the "home page" of a blog should be like the home page/front page of a news site, which always shows the latest/current headline, and not a chronological list of articles like an archive. It's not an exact analogy, since the blog only has one latest article (not a bunch every day like a newspaper), but I think you get the idea.

I modified the /posts page to remove the extra heading, and added a new link to the Blog menu for "All posts". I believe this addresses the mobile access concern, too.

I fixed broken link in the menu, which seems to have been a artifact of how I was linking it.

So, I think I addressed all the concerns you raised. LMK.

I'd love to see a few more styling changes, which I didn't get a chance to do... Otherwise I think we're getting close to being done:

  • In the "All posts" listing, it would be nice if the entire div containing the post excerpt was a link (not just the title).
  • In the article view, the title of the one you're viewing should be bolded in the sidebar listing
Screenshot 2024-07-05 at 1 54 39 PM

@chrisdel101
Copy link
Contributor Author

chrisdel101 commented Jul 15, 2024

  1. So that console issue seems to be related to retinaJS. There other instances in images with 2x in the folder seem to be used elsewhere. I've tried to find a way to stop this from erroring, like using the no-retina tag, but it doesn't work. I've spent some time on this and can't find any simple way to turn this off.

  2. remove the active class on the list of posts since we're no longer targeting the first post item.

  3. The subtitle issue seems to be fixed. So I deleted the conditional in page.html.

  4. Moved out the inline styles on the new image you added into the css file as a best practice. Hope this is okay.

  5. The image wasn't looking to good on mobile so I added some styles to fix that. Below is mobile and normal desktop
    Screenshot 2024-07-15 at 12 40 34 PM
    Screenshot 2024-07-15 at 12 41 01 PM

I'll address the rest of your issues in another commit.

I wanted to post something here to keep the progress from stalling. Now it's me whose sorry this is taking so long. I couldn't find a dev job so I'm spending 40 hours a week doing clerical work. (I'm finding it so much hardeh r to find time to contribute to FOSS after being locked out of tech).

@crandmck
Copy link
Member

crandmck commented Jul 16, 2024

No worries @chrisdel101, thanks for checking in... Sorry about your situation, but don't lose heart... I wish you best of luck and appreciate your contributions.

This all looks good. Let's make a final TO DO list, and then send this PR off! We can always add more polish, etc., in future PRs.

TO DO BEFORE MERGE:

  • Add bolding of post title in sidebar -- blog-post layout.
  • Remove 3 "dummy posts"
  • Change the date of _posts/2024-05-25-welcome-post.md to to current(ish) date and perhaps change the file name as well.
  • ?

LMK if you have any others.

- remove dumm posts
- move inline styles from post
- remove css comments
- add small margins
- Remove image
- undo css first child change
@chrisdel101
Copy link
Contributor Author

chrisdel101 commented Jul 17, 2024

I've made those changes and things are looking pretty good in the preview. A made a few other minor changes, mostly small amounts of margin, and moved some inline styles into the css file for maintainability.

If you see anything out of place let me know and I can fix it before it goes live.

I'll look for a fix to that retina error down the road.

@crandmck
Copy link
Member

I think this is ready to merge, thanks so much for all your work @chrisdel101 ....

@UlisesGascon I'm not sure why the check-translation test is failing? Can you help?

Initially, the blog will be English-only and IMO we shouldn't hold it up for i18n. I would just land it, but ideally would like to understand what's going on with this test failure.

@crandmck crandmck merged commit e4811a1 into expressjs:gh-pages Jul 22, 2024
6 of 7 checks passed
@crandmck
Copy link
Member

We'll figure out the failing tests later. Thanks for all your work on this @chrisdel101 !

@crandmck crandmck mentioned this pull request Jul 31, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants