Skip to content
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

Reorganize files related to splash #951

Merged
merged 8 commits into from
Jul 29, 2024
Merged

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Jul 10, 2024

preview

Description of proposed changes

Lots of small changes. See commit messages.

Related issue(s)

Checklist

@victorlin victorlin self-assigned this Jul 10, 2024
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-victorlin--s3vjsx July 10, 2024 19:00 Inactive
@victorlin victorlin force-pushed the victorlin/reorganize-splash branch from 5eec50f to d194760 Compare July 10, 2024 20:38
@nextstrain-bot nextstrain-bot temporarily deployed to nextstrain-s-victorlin--lzgmyt July 10, 2024 20:39 Inactive
@victorlin victorlin force-pushed the victorlin/reorganize-splash branch from d194760 to 8f022ab Compare July 10, 2024 20:42
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--lzgmyt July 10, 2024 20:42 Inactive
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--lzgmyt July 10, 2024 23:11 Inactive
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--lzgmyt July 16, 2024 21:35 Inactive
@@ -19,6 +18,21 @@ const NavContainer = styled.div`
left: 0px;
z-index: 1001;
transition: left .3s ease-out;

// TODO: Put this component in a container which sets these styles for all pages.
Copy link
Member

Choose a reason for hiding this comment

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

[for consideration] You could use a global CSS class (here are the global CSS imports we currently use) or a CSS module which both pages import.

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't actually tried so this could get complicated, but I would rather follow through here and create a common page-level container component to be used across all pages so that there's not so many different ones with varying styles and navbar + footer logos added separately.

Or maybe the split is intentional – if that's the case, this TODO doesn't really make sense and I'll consider using the global CSS class here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this change to #958

@victorlin victorlin force-pushed the victorlin/reorganize-splash branch from bc353f1 to 74a3882 Compare July 27, 2024 00:39
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--lzgmyt July 27, 2024 00:39 Inactive
Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

Reorganization makes sense to me 👍

That is the only usage. Also add a note on where it was copied from.
Components in this file are not used on Splash and should be directly
under src/components.
These are not used on Splash and should be directly under
src/components.
This sets an unknown type for all YAML imports, to be set later.
Part of a larger effort to add types in the codebase.
The old names were based on implementation details in the expansion of
tile usage from ListResources to Splash in these commits:

- "Separate Showcase component from ListResources" (1414985)
- "Add Showcase to Splash" (70cf0c6)
Part of a larger effort to move away from class components in the
codebase.
Part of a larger effort to add types in the codebase.
@victorlin victorlin force-pushed the victorlin/reorganize-splash branch from 74a3882 to fa40d2b Compare July 29, 2024 21:55
@victorlin victorlin temporarily deployed to nextstrain-s-victorlin--lzgmyt July 29, 2024 21:56 Inactive
@victorlin victorlin merged commit 7899940 into master Jul 29, 2024
7 checks passed
@victorlin victorlin deleted the victorlin/reorganize-splash branch July 29, 2024 22:02
@victorlin victorlin mentioned this pull request Jul 29, 2024
15 tasks
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.

4 participants