expressjs / expressjs.com

https://expressjs.com
Other
5.23k stars 1.42k forks source link

Express Blog #1519

Open chrisdel101 opened 1 month ago

chrisdel101 commented 1 month ago

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 :)

netlify[bot] commented 1 month ago

Deploy Preview for expressjscom-preview ready!

Name Link
Latest commit f063e4ac0925ee72a17eb4023b9f5e8bc3d59f88
Latest deploy log https://app.netlify.com/sites/expressjscom-preview/deploys/66785f28e477b400081ac5ca
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.

crandmck commented 1 month ago

This is great @chrisdel101 thanks for all the work! I think this PR will take a while to land, but it will be an excellent addition to the site.

Unfortunately, landing #1521 has caused some merge conflicts, which you'll need to fix. Sorry about that...

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.

I think /write-posts is sufficient, and looks good. You can omit /example-post IMO. I should probably put this inline, but "A local installation will be required it you want to preview your post" is no longer true since we now have Netlify previews on PRs. Could you please add that?

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?

It's fine as is IMO.

Also, I had in mind listing the posts somewhere, for example in the left gutter, under "Posts" but indented a bit? Then I guess "Write a post" would be at the top. LMK if that's unclear.

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.

I don't have access to a Windows machine, but maybe we can find someone who does before landing this.

The gutter menu is just another listing of the blog links.

All I see is: Posts Write a Post Example Post

It should list the titles of all the posts, e.g. Express And AI: Using LLMs, etc.

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.

In the long run, having tags will be useful so lets keep it for now. I didn't get a chance to review this part of the PR yet, but I will at some point.

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.

Yes, that makes total sense.

I think before landing this we should also have a first "real" post that's just a simple "Welcome to the Express blog" that you & I can write jointly. I'd like to explain the goals of the blog and explain that initially posts will be written by TC members (potentially with others), purely for the reason that we don't have bandwidth to review general posts from the broader community atm. Eventually, we want to, but for now the focus is on trying to release 5.0, and everyone just has finite time to work on the project. IMO others can potentially contribute to a post written by one or more TC members, (for example, this initial post). The @expressjs/express-tc discussed the blog previously, but I'd like to discuss the specifics a bit more the next time we have a chance.

For example: What are the goals of the blog?

I propose that it can be an avenue:

chrisdel101 commented 1 month ago

I fixed most of what that you suggested. And I added a Welcome Post. I wasn't totally sure what to write there, so feel free to critique that.

The contribution process content is not fixed. I don't really understand this. Can you explain how people will submit posts? It requires a branch/fork to PR from, so I'm not totally clear on that. Once you explain it I'll make that change.

One small issue on FF for mac. At a certain point in the resizing the search bar is squished. I'm not sure I can do much about this here, unless we want to do a navbar re-vamp. Can we get away with this? Screenshot 2024-05-26 at 12 23 04 PM

Screenshot 2024-05-26 at 12 23 11 PM

crandmck commented 1 month ago

Thanks @chrisdel101

Sorry I haven't had a chance to comment on this, just been slammed at work.

Re the content of the first post, would you mind if I edited it in your branch/fork? That would probably be the easiest way to collaborate on it.

Here is some background from earlier discussion: https://github.com/expressjs/expressjs.com/issues/1500#issuecomment-2089542343

The contribution process content is not fixed. I don't really understand this. Can you explain how people will submit posts? It requires a branch/fork to PR from, so I'm not totally clear on that. Once you explain it I'll make that change.

Basically, it's the same process as any PR. But I think we should ask people to open an issue first so the topic can be discussed. And only after it's been informally approved in the issue, then open a PR. What we don't want is a bunch of contributions of posts on random topics that require a lot of time for the TC to work on, as we have just don't have the bandwidth for that.

One small issue on FF for mac. At a certain point in the resizing the search bar is squished. I'm not sure I can do much about this here, unless we want to do a navbar re-vamp. Can we get away with this?

I think it's OK, because as you narrow the window the layout quickly changes to the narrow/mobile one with the hamburger menu. Also I wonder if this would obviate this problem: https://github.com/expressjs/expressjs.com/pull/1522 ?

One other small comment which I thought I mentioned before but just got lost along the way... In Write A Blog Post/ Setup the Repository:

A local installation will be required it you want to preview your post....

This is no longer true now that we have deploy previews. Once you open a PR you can see how it looks in the deploy preview.

chrisdel101 commented 4 weeks ago

So I've

Do no hesitate to point out errors or corrections :)

chrisdel101 commented 4 weeks ago

Also, I just pushed a new first line to the /write-post instructions. We might want to consider adding a blog-post to the issues types, if this becomes the accepted workflow.

  1. Propose your Post

Before taking the time to write a post, please confirm that we will be able to publish it. We're looking for topics related very specifically to Express, and so we want to pre-approve all posts. For the moment, this means we aren't excepting any unsolicited posts.

  • If you have an idea, please ask for approval first by opening an issue entitled Blog Post Idea <your idea>.
chrisdel101 commented 3 weeks ago

@crandmck Just added another bullet about how we can use the preview feature as a development tool for authors. Let me know if it's not going to work.

If you do not have the app running locally, this is where you will be able to preview your results. After you make your PR, there will be a section on the page entitled Deploy Preview for expressjscom-preview ready! Click here to see your work.

You can use this feature over multiple commits to refine your post, just make sure to open your pull request as a draft. Once it’s ready for review, switch it to a formal PR.

So as far as I can tell, there is no possible way to authors to write a post without forking the repo. The only step they do not need to complete, that a developer would, is actually getting the app running. They still need to have the code to make the PR, but can use this draft PR method to avoid running things locally.

chrisdel101 commented 3 weeks ago

Added updated content to Welcome Post based on #1500

Sorry to keep updating things. Keep finding improvements. Hopefully this will be the last.

Screenshot 2024-06-09 at 1 27 40 PM

crandmck commented 3 weeks ago

OK, it's coming along nicely... I edited the first post in your fork/branch. I removed the subtitle from this post, as it didn't seem to add anything.

One thing that I question is the requirement to have an image for each blog post. Images are great, but should they really be required?

chrisdel101 commented 2 weeks ago

I've removed the image. I think it has to be either/or considering our simple setup.

Re: sub title. You'll have noticed that removing the sub title makes the characters "##" render on the page. It would need logic code to block this. Example:

Screenshot 2024-06-15 at 12 26 03 PM

Without adding code to the post templates, which authors would also need to add, we cannot have optional fields (Technically it might be possible but probably not worth it for such a feature). The templates should probably be markdown only (i.e. not have any code in them)

So all fields are mandatory. I should prob put an instruction stating this in the guide

Do we want sub title removed overall then? Otherwise your post has to have it.

chrisdel101 commented 2 weeks ago

and i need to remove all the stuff related to image ( maybe subtitle) from the text still too. once you review it.

crandmck commented 1 week ago

Sorry it's taken me so long to get to this. Life...

Anyway, I made a few more commits to the fork/branch. As you can see, I updated the layout to do some of the work, so you no longer have to put the title & subtitle in two places, and subtitle is optional. I update the site config to set some defaults. I want to make a few more tweaks along these lines, but don't have time atm.

I'll try to work on it again within the next couple days. I think we're getting close, just a little more polish and it should be good for a first cut.

crandmck commented 1 week ago

@chrisdel101 Could you please resolve the merge conflicts in css/theme.css? Then we can see the changes in the deploy preview. I guess this is a result of some other PR that was recently merged ...

carlosstenzel commented 1 week ago

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

https://github.com/expressjs/expressjs.com/pull/1529

chrisdel101 commented 1 week ago

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.

chrisdel101 commented 1 week ago

lowered transition time and I think it looks better.

crandmck commented 2 days ago

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.