-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add Browse page with paginated publication index #252
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
base: main
Are you sure you want to change the base?
Conversation
Add a /browse page showing all publications with client-side pagination, sorting, and card-based layout. Add Browse nav link to header. Convert homepage Browse Articles button from search dialog trigger to /browse link.
❌ Deploy Preview for insightjournal failed.
|
There was a problem hiding this 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 adds a new /browse page to The Insight Journal that displays all publications in a paginated, sortable card-based layout. The implementation enables users to browse the full publication catalog with client-side sorting and pagination for a smooth user experience.
Changes:
- Added new /browse page with publication listing, client-side pagination (20 items per page), and multiple sort options (newest/oldest/title)
- Added Browse navigation link to header for easy discovery
- Converted homepage "Browse Articles" button from search dialog trigger to direct link to /browse page
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/pages/browse/index.astro | New browse page with publication data extraction, paginated card layout, sort controls, and theme-aware navigation |
| src/pages/index.astro | Updated Browse Articles button from search trigger to /browse link; removed associated event handler code |
| src/components/Header.astro | Added Browse navigation link between Search and About links |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </div> | ||
| <!-- Desktop search (hidden on mobile) --> | ||
| <div class="desktop-search-container ij-nav-search"> | ||
| <SearchDialog /> |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SearchDialog component is included twice on this page: once through the Header component (line 69) and once directly (line 84). Since the Header component already includes SearchDialog at the end of its template, including it again here creates duplicate dialog elements in the DOM. The JavaScript code in SearchDialog uses querySelector which will only interact with the first instance, making the second instance non-functional. Remove the direct import and usage of SearchDialog here since it's already provided by the Header component.
| <script> | ||
| // Theme-aware logo switching functionality | ||
| document.addEventListener('DOMContentLoaded', () => { | ||
| const updateLogoForTheme = () => { | ||
| const isDark = document.documentElement.classList.contains('wa-dark'); | ||
| const logoElement = document.querySelector('.theme-aware-logo') as HTMLImageElement; | ||
|
|
||
| if (logoElement) { | ||
| if (isDark) { | ||
| logoElement.src = '/assets/logo-insight-journal-square-dark.png'; | ||
| } else { | ||
| logoElement.src = '/assets/logo-insight-journal-square-light.png'; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| updateLogoForTheme(); | ||
|
|
||
| const observer = new MutationObserver((mutations) => { | ||
| mutations.forEach((mutation) => { | ||
| if (mutation.type === 'attributes' && mutation.attributeName === 'class') { | ||
| updateLogoForTheme(); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| observer.observe(document.documentElement, { | ||
| attributes: true, | ||
| attributeFilter: ['class'] | ||
| }); | ||
| }); | ||
| </script> |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The theme-aware logo switching functionality is duplicated across multiple pages including src/pages/index.astro (lines 167-199), src/pages/about.astro (lines 625-660), and here. This is a maintenance burden as any changes to the logic would need to be replicated in all locations. Consider extracting this into a shared script file or utility function that can be imported and reused across pages.
| // SPDX-License-Identifier: Apache-2.0 | ||
| import BasePage from '@awesome-myst/myst-awesome/layouts/BasePage.astro'; | ||
| import SearchDialog from '../../components/SearchDialog.astro'; |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import is no longer needed since SearchDialog should not be included directly on this page (it's already included via the Header component at line 69). Remove this import statement.
| import SearchDialog from '../../components/SearchDialog.astro'; |
Add a /browse page showing all publications with client-side pagination, sorting, and card-based layout. Add Browse nav link to header. Convert homepage Browse Articles button from search dialog trigger to /browse link.
fix local dev build on mac / brave