Skip to content

🪟 🎨 Add Text component #15570

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

Merged
merged 12 commits into from
Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions airbyte-webapp/src/components/base/Text/Text.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import classNames from "classnames";
import React from "react";

import headingStyles from "./heading.module.scss";
import textStyles from "./text.module.scss";

type TextSize = "sm" | "md" | "lg";
type HeadingSize = TextSize | "xl";
type TextElementType = "p" | "span" | "div";
type HeadingElementType = "h1" | "h2" | "h3" | "h4" | "h5" | "h6";

interface BaseProps {
className?: string;
centered?: boolean;
}

interface TextProps extends BaseProps {
as?: TextElementType;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was inspired by styled components to call this as but is the name clear? Alternatively, I thought of element or type as well. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

as feels fairly standard, though some other opinions would definitely be useful :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I like as here

Copy link
Contributor

Choose a reason for hiding this comment

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

++ for as as well, as it feels pretty standard as of today.

size?: TextSize;
bold?: boolean;
}

interface HeadingProps extends BaseProps {
as: HeadingElementType;
size?: HeadingSize;
}

const getTextClassNames = ({ size, centered, bold }: Required<Pick<TextProps, "size" | "centered" | "bold">>) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Required<Pick<TextProps, "size" | "centered" | "bold">>

To confirm my understanding - the purpose of this is basically to remove the as and className fields from TextProps, because we don't actually want those to be class names on the component that we are returning below, correct?

If so, would it potentially be better to do something like Required<Omit<TextProps, "className" | "as">> instead, so that if we add more classname props to those types in the future we don't need to also edit these function signatures? Probably not a huge deal either way

Copy link
Contributor

Choose a reason for hiding this comment

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

You'd need to add the new property to the destructure anyway.
No matter what method, there's type safety if size, centered, or bold are changed in TextProps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case that the props were to expand, this would continue to work with Pick over using Omit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good point that the new props would need to be added to the destructure, I forgot about that. This looks good to me then!

const sizes: Record<TextSize, string> = {
sm: textStyles.sm,
md: textStyles.md,
lg: textStyles.lg,
};

return classNames(textStyles.text, sizes[size], centered && textStyles.centered, bold && textStyles.bold);
};

const getHeadingClassNames = ({ size, centered }: Required<Pick<HeadingProps, "size" | "centered">>) => {
const sizes: Record<HeadingSize, string> = {
sm: headingStyles.sm,
md: headingStyles.md,
lg: headingStyles.lg,
xl: headingStyles.xl,
};

return classNames(headingStyles.heading, sizes[size], centered && headingStyles.centered);
};

const isHeadingType = (props: TextProps | HeadingProps): props is HeadingProps => {
return props.as ? /^h[0-6]$/.test(props.as) : false;
};

export const Text: React.FC<TextProps | HeadingProps> = React.memo((props) => {
const isHeading = isHeadingType(props);
const { as = "p", centered = false, children } = props;

const className = classNames(
isHeading
? getHeadingClassNames({ centered, size: props.size ?? "md" })
: getTextClassNames({ centered, size: props.size ?? "md", bold: props.bold ?? false }),
props.className
);

return React.createElement(as, {
className,
children,
});
});
36 changes: 36 additions & 0 deletions airbyte-webapp/src/components/base/Text/heading.module.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
@use "../../../scss/colors";

.heading {
font-weight: 700;

color: colors.$dark-blue;
margin: 0;
padding: 0;
}

.sm {
font-size: 16px;
line-height: 1.2em;
}

.md,
.lg,
.xl {
line-height: 1.3em;
}

.md {
font-size: 20px;
}

.lg {
font-size: 24px;
}

.xl {
font-size: 32px;
}

.centered {
text-align: center;
}
17 changes: 17 additions & 0 deletions airbyte-webapp/src/components/base/Text/index.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { ComponentStory, ComponentMeta } from "@storybook/react";

import { Text } from "./Text";

export default {
title: "Ui/Text",
component: Text,
} as ComponentMeta<typeof Text>;

const Template: ComponentStory<typeof Text> = (args) => <Text {...args} />;

export const Primary = Template.bind({});
Primary.args = {
size: "md",
children:
"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.",
};
1 change: 1 addition & 0 deletions airbyte-webapp/src/components/base/Text/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { Text } from "./Text";
36 changes: 36 additions & 0 deletions airbyte-webapp/src/components/base/Text/text.module.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
@use "../../../scss/colors";

.text {
color: colors.$dark-blue;
margin: 0;
padding: 0;
}

.sm,
.md {
font-weight: 400;
line-height: 1.2em;
}

.sm {
font-size: 12px;
}

.md {
font-size: 13px;
}

.lg {
font-size: 14px;
font-weight: 500;
line-height: 1.3em;
}

.centered {
text-align: center;
}

.bold,
.text > strong {
font-weight: 600;
}
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
@use "../../../../../scss/colors";
@use "../../../../../scss/variables";

.title {
font-weight: 700;
font-size: 32px;
line-height: 39px;
width: 250px;
color: colors.$dark-blue-900;
margin-bottom: variables.$spacing-md;
}

.highlight {
color: colors.$blue-400;
color: colors.$blue;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from "react";
import { FormattedMessage } from "react-intl";

import { Text } from "components/base/Text";
import HeadTitle from "components/HeadTitle";

import { OAuthLogin } from "../OAuthLogin";
Expand All @@ -16,7 +17,7 @@ const SignupPage: React.FC<SignupPageProps> = ({ highlightStyle }) => {
return (
<div>
<HeadTitle titles={[{ id: "login.signup" }]} />
<h1 className={styles.title}>
<Text as="h1" size="xl" className={styles.title}>
<FormattedMessage
id="login.activateAccess"
values={{
Expand All @@ -27,7 +28,7 @@ const SignupPage: React.FC<SignupPageProps> = ({ highlightStyle }) => {
),
}}
/>
</h1>
</Text>
<SpecialBlock />
<SignupForm />
<OAuthLogin isSignUpPage />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Form as FormikForm } from "formik";
import styled from "styled-components";

export const Form = styled(FormikForm)`
margin-top: 42px;
margin-top: 40px;
`;

export const FieldItem = styled.div`
Expand All @@ -28,7 +28,7 @@ export const BottomBlock = styled.div`
flex-direction: row;
justify-content: space-between;
align-items: center;
margin-top: 38px;
margin-top: 40px;
font-size: 11px;
`;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
@use "../../../../../../scss/colors";

.title {
font-weight: 700;
font-size: 32px;
line-height: 39px;
color: colors.$blue-400;
color: colors.$blue;
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import React from "react";

import { Text } from "components/base/Text";

import styles from "./FormTitle.module.scss";

export const FormTitle: React.FC = ({ children }) => <h1 className={styles.title}>{children}</h1>;
export const FormTitle: React.FC = ({ children }) => (
<Text as="h1" size="xl" className={styles.title}>
{children}
</Text>
);