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

feat: Redesign the landing page #401

Open
wants to merge 4 commits into
base: site-redesign
Choose a base branch
from

Conversation

EdwardPrado
Copy link

Describe your changes

Updated the landing page to match the Figma Redesign which is referenced in #70.

  • Simple Icons package was added since Bootstrap icons didn't have all the social icons needed
  • Updated constants to have consistent uppercase
  • Redesign includes mobile support and dropdown navbar

Related issue number/link

Fixes #385

package.json Show resolved Hide resolved
src/routes/+layout.svelte Outdated Show resolved Hide resolved
@VeckoTheGecko
Copy link
Collaborator

VeckoTheGecko commented Aug 15, 2024

Looks good! Very excited for this. Just a note:

  • In light mode, the contrast between the text and the hero image is a bit low making it difficult to read. In the design I think the opacity of the hero image started fading sooner than in this implementation. Could you patch that?

Other than that, I'll leave it up to you @dmlb to approve/merge this since I think you have more idea of what's going on in the code here :)

>
<DarkModeControl cssClass="w-fit p-2"></DarkModeControl>

<button on:click={handleDropdownClick} id="menu">
Copy link
Contributor

Choose a reason for hiding this comment

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

one thing missing here, we need an accessible name for this menu button
Screen Shot 2024-08-21 at 4 13 52 PM

<a href="{base}/resources" class="heroCardContainer">
<div class="heroCard">
<div class="relative">
<img
Copy link
Contributor

Choose a reason for hiding this comment

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

we should circle back around later with some avif options 🚀

or
<a href={github_url} class="underline">edit on GitHub directly</a>
<div class="max-w-7xl mx-auto">
<h1 class="font-bold text-5xl mx-6">
Copy link
Contributor

Choose a reason for hiding this comment

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

design wise on mobile, looks like the headings should shrink some for that context
Screen Shot 2024-08-21 at 4 15 51 PM

{#if href}
<a
{href}
class="ctaBtn bg-transparent hover:bg-blue-tertiary text-white py-2 px-4 rounded-full mx-6 sm:mr-6"
Copy link
Contributor

Choose a reason for hiding this comment

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

for consistency with the tailwind usage and buttons, if you have the time to turn button class into something global with tailwind's @apply?

example:

.button {
    @apply bg-transparent hover:bg-blue-tertiary text-white border border-current py-2 px-4 rounded-full mx-6 sm:mr-6;

}

}

.ctaBtn:focus-visible {
box-shadow: 0 0 0 8px theme("colors.blue-primary");
Copy link
Contributor

@dmlb dmlb Aug 21, 2024

Choose a reason for hiding this comment

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

both the hover and the box-shadow currently match the background of the only use case and can't see any feedback changes from these styles 😅

Screen.Recording.2024-08-21.at.4.28.30.PM.mov

if the goal is just the size shift -- using transform: scale would be more performant

Copy link
Contributor

@dmlb dmlb left a comment

Choose a reason for hiding this comment

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

Looks good! excited to see the other pages - some small things

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.

3 participants