Skip to content

feat: Show image and other news on news pages #166

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

Merged
merged 7 commits into from
Jun 3, 2025
Merged

Conversation

MoritzWeber0
Copy link
Member

@MoritzWeber0 MoritzWeber0 commented May 30, 2025

On news pages, show the image and other news pages. On mobile devices, only the image is displayed.
While changing some parts, I refactored some code to use pure SCSS instead of bootstrap.

Resolves #144

@MoritzWeber0 MoritzWeber0 requested a review from therobrob May 30, 2025 06:22
Copy link

netlify bot commented May 30, 2025

Deploy Preview for fipguide ready!

Name Link
🔨 Latest commit 8ba4727
🔍 Latest deploy log https://app.netlify.com/projects/fipguide/deploys/683e9880b467b00008284d82
😎 Deploy Preview https://deploy-preview-166--fipguide.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 project configuration.

@MoritzWeber0 MoritzWeber0 requested review from Copilot and therobrob and removed request for therobrob May 30, 2025 06:22
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a new feature for news pages by showing an image and additional news items, and it includes a refactor from Bootstrap-based layout to custom SCSS. Key changes include new templates for news pages, layout adjustments for operator and country pages, and style updates using native media queries.

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
layouts/partials/internal_link.html Added simple, self-contained internal link helper
layouts/operator/single.html Updated layout structure replacing Bootstrap grid
layouts/news/single.html New news page template displaying image and news list
layouts/news/list.html Introduced list view for news teasers
layouts/country/single.html Modified layout for country pages with simplified markup
layouts/_default/single.html Adjusted default template structure
i18n/en.yaml & i18n/de.yaml Updated translations for news-related keys
assets/sass/* Refactored SCSS to use native @media queries and grid layout
CONTRIBUTING.md Updated dependency reference from Bootstrap to pagefind

@MoritzWeber0 MoritzWeber0 force-pushed the feat/news-splitview branch 2 times, most recently from 23ad3a7 to ab04bfa Compare May 30, 2025 06:40
@MoritzWeber0 MoritzWeber0 force-pushed the feat/news-splitview branch from ab04bfa to 2d855be Compare May 30, 2025 06:44
lenderom
lenderom previously approved these changes May 30, 2025
@therobrob
Copy link
Member

therobrob commented May 31, 2025

Thanks for refactoring the sidebar, i like the new presentation of the single news!! :) – as mentioned in the files already, we should ask ourselves whether we should also use the bem-logic for consistency in new code. :) i´ve made some comments, it would be cool to take a look :)

@MoritzWeber0
Copy link
Member Author

MoritzWeber0 commented May 31, 2025

Thanks for refactoring the sidebar, i like the new presentation of the single news!! :) – as mentioned in the files already, we should ask ourselves whether we should also use the bem-logic for consistency in new code. :) i´ve made some comments, it would be cool to take a look :)

Thanks for the good review and being picky about applying BEM. I support the efforts to keep the code consistent and updated the newly introduced classes to BEM logic. Not 100% sure about .o-news_list. Can you take another look?

@MoritzWeber0 MoritzWeber0 requested a review from therobrob May 31, 2025 17:53
@MoritzWeber0 MoritzWeber0 force-pushed the feat/news-splitview branch from 81d7f02 to d692c81 Compare May 31, 2025 20:25
@MoritzWeber0 MoritzWeber0 marked this pull request as draft May 31, 2025 20:27
@MoritzWeber0 MoritzWeber0 force-pushed the feat/news-splitview branch from d692c81 to c4b0071 Compare May 31, 2025 20:30
@MoritzWeber0 MoritzWeber0 marked this pull request as ready for review May 31, 2025 20:32
@MoritzWeber0 MoritzWeber0 force-pushed the feat/news-splitview branch from c4b0071 to 310f870 Compare June 1, 2025 13:34
@MoritzWeber0 MoritzWeber0 force-pushed the feat/news-splitview branch from 310f870 to ac4de6d Compare June 2, 2025 18:27
@lenderom
Copy link
Member

lenderom commented Jun 2, 2025

@therobrob can we merge this?

@therobrob
Copy link
Member

@lenderom not yet.

…ews-splitview

# Conflicts:
#	assets/sass/sidemenu.scss
#	assets/sass/styles.scss
#	layouts/country/single.html
#	layouts/news/single.html
#	layouts/operator/single.html
@therobrob therobrob merged commit 93c9027 into main Jun 3, 2025
7 checks passed
@therobrob therobrob deleted the feat/news-splitview branch June 3, 2025 06:42
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.

News-Image in Detail View
3 participants