-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(website): refactor footer to pull "download data" button closer to React-rendered content #978
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: 981-migrate-covid-compare-side-by-side-page-to-react
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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 refactors the footer structure to move the "download data" button and related components closer to React-rendered content in preparation for rendering that portion of the footer in React. The refactoring removes the SecondaryFooter component and relocates footer content from the layout level to individual pages.
Key changes:
- Removed the
SecondaryFootercomponent and its slot-based rendering - Moved footer content (DataInfo, download buttons, last updated info) from layout templates into page-specific implementations
- Restructured the DOM hierarchy to use flexbox layouts with
h-screenandh-fullfor proper vertical spacing
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| website/src/pages/index.astro | Moved DataInfo directly into page content with border styling |
| website/src/layouts/base/footer/SecondaryFooter.astro | Deleted entire component file |
| website/src/layouts/base/footer/Footer.astro | Removed SecondaryFooter import and slot usage |
| website/src/layouts/base/BaseLayout.astro | Changed body from min-h-screen to h-screen, moved Header outside of grow container, removed secondary-footer slot |
| website/src/layouts/OrganismPage/SingleVariantOrganismPageLayout.astro | Removed gs-app wrapper and LAPIS configuration (moved to parent) |
| website/src/layouts/OrganismPage/OrganismViewPageLayout.astro | Added gs-app wrapper, footer components (AccessionsDownloadButton, LastUpdatedInfo, DataInfo) with flexbox layout |
| website/src/layouts/OrganismPage/DataPageLayout.astro | Removed footer-related props and rendering, added flexbox container structure |
| website/src/components/views/compareSideBySide/GenericCompareSideBySidePage.astro | Removed gs-app wrapper (now in parent component) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </head> | ||
|
|
||
| <body class='flex min-h-screen flex-col'> | ||
| <body class='flex h-screen flex-col'> |
Copilot
AI
Dec 17, 2025
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.
Changing from min-h-screen to h-screen restricts the body height to exactly the viewport height. This will cause content overflow issues on pages with content taller than the viewport, preventing users from scrolling to see all content. Consider keeping min-h-screen or ensure all pages using this layout can fit within a single viewport.
| <body class='flex h-screen flex-col'> | |
| <body class='flex min-h-screen flex-col'> |
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 body needs a defined height so that h-full inside flex and grow works. The content does overflow the body, but it looks like it works, so I assume that this is fine?
370843f to
a23a26a
Compare
| --- | ||
|
|
||
| <OrganismViewPageLayout view={view} downloadLinks={downloadLinks}> | ||
| <gs-app lapis={getLapisUrl(view.organismConstants.organism)}> |
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.
I deleted this line (moved to OrganismViewPageLayout) and reformatted the rest.
58b83f5 to
6bbe89b
Compare
…ser to React-rendered content so that in a next step, that part of the footer can actually be rendered in React (since it depends on the page state) resolves #977
6bbe89b to
98d0fc4
Compare
resolves #977
Summary
so that in a next step, that part of the footer can actually be rendered in React (since it depends on the page state)
Screenshot
PR Checklist
- [ ] All necessary documentation has been adapted.- [ ] The implemented feature is covered by an appropriate test.