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

Case studies layout #46

Merged
merged 15 commits into from
Nov 13, 2024
Merged

Case studies layout #46

merged 15 commits into from
Nov 13, 2024

Conversation

jakewheeler
Copy link
Collaborator

@jakewheeler jakewheeler commented Nov 8, 2024

This PR creates an initial layout for the "case studies" page.

Here's a sample image of the current layout:
image

@jakewheeler jakewheeler changed the base branch from main to next November 8, 2024 14:09
@jakewheeler jakewheeler marked this pull request as ready for review November 13, 2024 15:21
Copy link
Collaborator

Choose a reason for hiding this comment

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

These case studies are pretty much all the same so we could just make this a function and have the content be a data array.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was my plan initially, and at one point I began creating reusable components to build the layout so that we could do something similar to your suggestion. The main issue with this was that it started to become painful when styling changes didn't apply throughout the entire layout.

For example, in order to match the design the image container in section 3 has lg:ml-4 but none of the other image containers do. The styling also changes again slightly depending on whether the image is on the left or right.

I found it tedious and more confusing to manage to try to include a lot of options for a reusable component in order to get this working. What do you think? Anything else I cant try?

disabled = false,
...props
}: LinkButtonProps) {
if (variant === 'secondary') {
return (
<Link
href={href}
className="usa-button hover:bg-red items-center justify-start gap-2.5 rounded border-2 border-[#224a58] bg-white px-5 py-3 hover:border-2"
className={classNames(
'usa-button hover:bg-red items-center gap-2.5 rounded border-2 border-[#224a58] bg-white px-5 py-3 hover:border-2',
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should refactor these class name strings into data objects. Then we can just set the classes with that.

So we can have a disabled button return,

then a another one, and set the classes based on if the variant is secondary

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you give me an example of what you envision the "data object" to look like? Just want to make sure I get what you mean.

Copy link
Collaborator

Choose a reason for hiding this comment

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

import classNames from 'classnames';
import Link, { LinkProps } from 'next/link';

const BUTTON_STYLES = {
  primary: {
    button: 'usa-button bg-[#224a58] hover:bg-green',
    text: 'text-white',
  },
  secondary: {
    button:
      'usa-button hover:bg-red items-center gap-2.5 rounded border-2 border-[#224a58] bg-white px-5 py-3 hover:border-2',
    text: 'text-[#224a58]',
  },
} as const;

interface LinkButtonProps extends LinkProps {
  children: React.ReactNode;
  variant: 'primary' | 'secondary';
  className?: string;
  disabled?: boolean;
}
interface LinkButtonProps extends LinkProps {
  children: React.ReactNode;
  variant: 'primary' | 'secondary';
  className?: string;
  disabled?: boolean;
  'aria-label'?: string;
}
export function LinkButton({
  children,
  variant,
  href,
  className = '',
  disabled = false,
  'aria-label': ariaLabel,
  ...props
}: LinkButtonProps) {
  if (disabled) {
    return (
      <div
        className="usa-button usa-button--disabled self-end justify-self-start bg-[#224a58]"
        aria-disabled="true"
        role="link"
        aria-label={ariaLabel}
      >
        <span className="text-base font-bold text-black">{children}</span>
      </div>
    );
  }

  return (
    <Link
      href={href}
      className={classNames(BUTTON_STYLES[variant].button, className)}
      aria-label={ariaLabel}
      {...props}
    >
      <span
        className={classNames(
          'text-center text-base font-bold',
          BUTTON_STYLES[variant].text,
        )}
      >
        {children}
      </span>
    </Link>
  );
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like it. Thanks!

@jakewheeler jakewheeler merged commit b3046ac into next Nov 13, 2024
1 check passed
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.

2 participants