Skip to content

[FEAT] Don't display all the posts in the 'News' section of the 'Home' page#229

Merged
csouchet merged 5 commits into
mainfrom
187-Don_t_display_all_the_news_in_the_home_page
Nov 22, 2021
Merged

[FEAT] Don't display all the posts in the 'News' section of the 'Home' page#229
csouchet merged 5 commits into
mainfrom
187-Don_t_display_all_the_news_in_the_home_page

Conversation

@csouchet

@csouchet csouchet commented Nov 19, 2021

Copy link
Copy Markdown
Contributor

Covers #187

@csouchet csouchet added enhancement New feature or request depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first labels Nov 19, 2021
@github-actions

github-actions Bot commented Nov 19, 2021

Copy link
Copy Markdown

♻️ PR Preview a0dbf1b has been successfully destroyed since this PR has been closed.

🤖 By surge-preview

@csouchet csouchet changed the title [FEAT] Don't display all the posts in the 'News' section of the Home page [FEAT] Don't display all the posts in the 'News' section of the 'Home' page Nov 19, 2021
@csouchet
csouchet force-pushed the 187-Don_t_display_all_the_news_in_the_home_page branch 2 times, most recently from f7a270c to e8444b2 Compare November 22, 2021 14:29
@csouchet
csouchet force-pushed the 187-Don_t_display_all_the_news_in_the_home_page branch from e8444b2 to 58c2191 Compare November 22, 2021 14:44
@csouchet csouchet removed the depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first label Nov 22, 2021
@csouchet
csouchet marked this pull request as ready for review November 22, 2021 14:45
@csouchet
csouchet requested a review from tbouffard November 22, 2021 14:45

@tbouffard tbouffard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, a minor comment that doesn't block the merge

containerTitle={SECTION.news}
Background={Background}
posts={newsContent.news}
pageId="news"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

⚠️ for now we have less than 7 news, so directing the 'news' page doesn't provide new content.
However, I don't think it's worth taking time to hide the 'More' button we we have less than 7 elements in the list. We should put energy on writing more news to drop the need for hiding the button 😈
Feel free to merge without changing anything here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@csouchet
csouchet merged commit 41a4712 into main Nov 22, 2021
@csouchet
csouchet deleted the 187-Don_t_display_all_the_news_in_the_home_page branch November 22, 2021 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants