[Blog] Feature: Post Archives #2356
Conversation
|
Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Newcomers' Guide and sure to join the community Slack. |
✅ Deploy Preview for mesheryio-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary of ChangesHello @Devansh422, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new 'Browse by date' feature to the blog's sidebar, significantly enhancing navigation for users. It implements a structured, collapsible archive that organizes blog posts hierarchically by year and month. The changes encompass both the Liquid templating logic required for dynamic content generation and the necessary CSS to ensure proper presentation and user interaction for this new feature. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a 'Browse by date' feature to the blog sidebar, allowing users to navigate posts grouped by year and month. The implementation has some critical issues with HTML validity and uses a complex logic for grouping. I've provided a detailed comment with a suggested refactoring that uses Jekyll's built-in filters to simplify the code, ensure correctness, and improve maintainability. The accompanying CSS changes look good.
| <div class="sidebar-section"> | ||
| <h3 style="font-size:1.5rem;text-align: center;margin:auto;">Browse by date</h3> | ||
| <ul class="archive-index"> | ||
| {% assign prev_year = '' %} | ||
| {% assign prev_month = '' %} | ||
| {% for post in site.posts %} | ||
| {% assign year = post.date | date: '%Y' %} | ||
| {% assign month = post.date | date: '%B' %} | ||
|
|
||
| {% if year != prev_year %} | ||
| {% if prev_year != '' %} | ||
| </ul></details> | ||
| </ul></details> | ||
| {% endif %} | ||
| <details class="archive-year" {% if forloop.first %}open{% endif %}> | ||
| <summary>{{ year }}</summary> | ||
| <ul class="archive-months"> | ||
| {% assign prev_month = '' %} | ||
| {% endif %} | ||
|
|
||
| {% if month != prev_month %} | ||
| {% if prev_month != '' %} | ||
| </ul></details> | ||
| {% endif %} | ||
| <details class="archive-month"> | ||
| <summary>{{ month }}</summary> | ||
| <ul class="archive-posts"> | ||
| {% endif %} | ||
|
|
||
| <li><a href="{{ post.url | relative_url }}">{{ post.title }}</a></li> | ||
|
|
||
| {% assign prev_year = year %} | ||
| {% assign prev_month = month %} | ||
|
|
||
| {% if forloop.last %} | ||
| </ul></details> | ||
| </ul></details> | ||
| {% endif %} | ||
| {% endfor %} | ||
| </ul> | ||
| </div> |
There was a problem hiding this comment.
The current implementation for generating the archive list has a couple of issues that lead to invalid HTML and is overly complex, making it hard to maintain.
- Invalid HTML Structure: The
<details>elements for years and months are placed as direct children of<ul>elements (archive-indexandarchive-months). According to HTML standards,<ul>elements should only contain<li>elements as direct children. This can cause rendering issues in some browsers. - Complex and Fragile Logic: The logic relies on manually tracking year and month changes (
prev_year,prev_month) and manually opening/closing HTML tags. This is error-prone and difficult to read and debug.
A much cleaner, more robust, and idiomatic way to achieve this in Jekyll is to use the group_by_exp filter. This simplifies the logic immensely by handling the grouping for you, resulting in more readable and maintainable code that produces valid HTML.
Here is a suggested replacement for the entire block which addresses these issues.
<div class="sidebar-section">
<h3 style="font-size:1.5rem;text-align: center;margin:auto;">Browse by date</h3>
<ul class="archive-index">
{% assign posts_by_year = site.posts | group_by_exp: "post", "post.date | date: '%Y'" | sort: 'name' | reverse %}
{% for year_group in posts_by_year %}
<li>
<details class="archive-year" {% if forloop.first %}open{% endif %}>
<summary>{{ year_group.name }}</summary>
<ul class="archive-months">
{% assign posts_by_month = year_group.items | group_by_exp: "post", "post.date | date: '%m'" | sort: 'name' | reverse %}
{% for month_group in posts_by_month %}
<li>
<details class="archive-month">
<summary>{{ month_group.items.first.date | date: '%B' }}</summary>
<ul class="archive-posts">
{% for post in month_group.items %}
<li><a href="{{ post.url | relative_url }}">{{ post.title }}</a></li>
{% endfor %}
</ul>
</details>
</li>
{% endfor %}
</ul>
</details>
</li>
{% endfor %}
</ul>
</div>
|
@leecalcote |
|
@Devansh422 Hey Kindly sign off your commits To add your Signed-off-by line to every commit in this branch:
|
Signed-off-by: Devansh422 <[email protected]>
Devansh422
force-pushed
the
feature-blog
branch
from
3b2ee34 to
812e0a1
Compare
|
i followed the steps you guided |
Signed-off-by: Devansh422 <[email protected]>
Signed-off-by: Devansh422 <[email protected]>
Devansh422
force-pushed
the
feature-blog
branch
from
be0a91e to
4c59f25
Compare
|
@Rajesh-Nagarajan-11 @leecalcote @saurabhraghuvanshii there was an UI issue in the side bar 
I updated the css of blog page to resolve this 
|
|
And also consider Gemini's review to more optimize and instead of rebase your repo for every commits use |
Signed-off-by: Devansh422 <[email protected]>
|
@Rajesh-Nagarajan-11 @leecalcote @saurabhraghuvanshii i have fixed the ui of the listing of the list |
|
Thanks @Devansh422 lgtm, did you check for other page if this changes working well on other blogs. |
|
Hey!, @saurabhraghuvanshii, i have checked the pages. its working well on all pages |
|
@Rajesh-Nagarajan-11 |
|
@leecalcote Please check the PR and merge |
|
@Rajesh-Nagarajan-11 @saurabhraghuvanshii what is the process of merging the PR |
Namanv0509
added
the
hacktoberfest-accepted
|
@Devansh422 can you look over this issue The box is not correctly sizes also 
The blogs should be seprated by a horizontal line or a box border to distinguish them |
|
@Devansh422 Make sure to present them in websites call , thanks for taking up on this effort |
|
@Devansh422 I dont think this is desired , the filter should work same as categories , a different section to filter with date. As we can check by year or month . with year having months as drop down , or they can check the year to get those filtered blog for that year . |
|
@Devansh422, almost there. This formatting needs to be addressed. Is there a dedicated "Archives" page? |
|
In the future, offer a descriptive PR title, please, @Devansh422 |
|
@Devansh422, will you be finalizing this PR? |
|
No movement from @Devansh422. @Namanv0509 please finish off this PR. |
Signed-off-by: Naman Verma <[email protected]>
|
Thanks for your contribution to the Meshery community! 🎉
|
5 participants
Description
This PR fixes #2355
Notes for Reviewers
Signed commits