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

coreui NoteBox component #1276

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
30 changes: 27 additions & 3 deletions packages/libs/coreui/src/components/containers/NoteBox.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
import { css } from '@emotion/react';
import React, { ReactNode } from 'react';
import { error, warning, mutedBlue } from '../../definitions/colors';
import { Error, Info, Warning } from '@material-ui/icons';

export interface Props {
type: 'info' | 'warning' | 'error';
showIcon?: boolean;
children: ReactNode;
}

const baseCss = css`
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps additionally a default font? I believe coreui generally uses Inter or Roboto

Copy link
Member Author

Choose a reason for hiding this comment

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

For content, I think we should use the same font as the rest of the site, but I will take a look at using one of these other fonts.

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 think I'm going to leave the font as-is.

border-left: 0.25em solid var(--note-box-border-color);
border-left: 0.35em solid var(--note-box-border-color);
border-radius: 0.25em;
padding: 0.5em 1em;
padding: 1em 3em;
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: have padding change based on showIcon.

background: var(--note-box-bg-color);
gap: 1em;
margin-bottom: 1em;
position: relative;
`;

const infoCss = css`
Expand All @@ -36,12 +39,33 @@ const errorCss = css`
${baseCss};
`;

const iconCss = css`
position: absolute;
left: 0.5em;
font-size: 1.5em;
`;
Copy link
Member

Choose a reason for hiding this comment

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

What do you think of adding color: var(--note-box-border-color); ?
I tried it out and thought it looked pretty good!


export function NoteBox(props: Props) {
const finalCss =
props.type === 'warning'
? warningCss
: props.type === 'error'
? errorCss
: infoCss;
return <div css={finalCss}>{props.children}</div>;

const Icon =
props.showIcon === true
? props.type === 'info'
? Info
: props.type === 'warning'
? Warning
: props.type === 'error'
? Error
: null
: null;
return (
<div css={finalCss}>
{Icon ? <Icon css={iconCss} /> : null} {props.children}
</div>
);
}
110 changes: 96 additions & 14 deletions packages/libs/coreui/src/stories/containers/NoteBox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,25 @@ export default {

const Template: Story<Props> = function Template(props) {
return (
<UIThemeProvider
theme={{
palette: {
primary: { hue: mutedGreen, level: 500 },
secondary: { hue: mutedMagenta, level: 500 },
},
}}
>
<NoteBox {...props} />
</UIThemeProvider>
<div style={{ fontFamily: 'sans-serif' }}>
<UIThemeProvider
theme={{
palette: {
primary: { hue: mutedGreen, level: 500 },
secondary: { hue: mutedMagenta, level: 500 },
},
}}
>
<NoteBox {...props} />
</UIThemeProvider>
</div>
);
};

export const Info = Object.assign(Template.bind({}), {
export const InfoWithoutIcon = Object.assign(Template.bind({}), {
args: {
type: 'info',
showIcon: false,
children: (
<div>
This is some general information about the content that follows on the
Expand All @@ -36,18 +39,20 @@ export const Info = Object.assign(Template.bind({}), {
},
});

export const Warning = Object.assign(Template.bind({}), {
export const WarningWithoutIcon = Object.assign(Template.bind({}), {
args: {
type: 'warning',
showIcon: false,
children: (
<div>This is a warning about the content that follows on the page.</div>
),
},
});

export const Error = Object.assign(Template.bind({}), {
export const ErrorWithoutIcon = Object.assign(Template.bind({}), {
args: {
type: 'error',
showIcon: false,
children: (
<div>
This is an error message about the content that follows on the page.
Expand All @@ -56,9 +61,10 @@ export const Error = Object.assign(Template.bind({}), {
},
});

export const LongContent = Object.assign(Template.bind({}), {
export const LongContentWithoutIcon = Object.assign(Template.bind({}), {
args: {
type: 'info',
showIcon: false,
children: (
<div>
Lorem ipsum odor amet, consectetuer adipiscing elit. Faucibus morbi ac
Expand All @@ -72,3 +78,79 @@ export const LongContent = Object.assign(Template.bind({}), {
),
},
});
Copy link
Member

Choose a reason for hiding this comment

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

A story using a different type of child (for example, an image or some other non-text element) would be helpful if time allows

Copy link
Member

Choose a reason for hiding this comment

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

or an expandable one

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I agree! I will add a couple more, soon.


export const InfoWithIcon = Object.assign(Template.bind({}), {
args: {
type: 'info',
showIcon: true,
children: (
<div>
This is some general information about the content that follows on the
page.
</div>
),
},
});

export const WarningWithIcon = Object.assign(Template.bind({}), {
args: {
type: 'warning',
showIcon: true,
children: (
<div>This is a warning about the content that follows on the page.</div>
),
},
});

export const ErrorWithIcon = Object.assign(Template.bind({}), {
args: {
type: 'error',
showIcon: true,
children: (
<div>
This is an error message about the content that follows on the page.
</div>
),
},
});

export const LongContentWithIcon = Object.assign(Template.bind({}), {
args: {
type: 'info',
showIcon: true,
children: (
<div>
Lorem ipsum odor amet, consectetuer adipiscing elit. Faucibus morbi ac
ultrices purus urna tristique mattis consequat. Posuere volutpat
facilisi natoque dictumst dignissim magna dapibus. Taciti vel a etiam
curabitur velit torquent. Fusce interdum dictum vulputate sollicitudin
nulla. Orci placerat congue odio aptent enim mauris. Turpis nec rhoncus
eleifend eleifend eget. Auctor sed nullam vestibulum quisque egestas;
nullam aenean ante.
</div>
),
},
});

export const ExpandableContentWithIcon = Object.assign(Template.bind({}), {
args: {
type: 'info',
showIcon: true,
children: (
<details>
<summary style={{ cursor: 'pointer' }}>
There are some interesting things about this...
</summary>
<p>
Lorem ipsum odor amet, consectetuer adipiscing elit. Faucibus morbi ac
ultrices purus urna tristique mattis consequat. Posuere volutpat
facilisi natoque dictumst dignissim magna dapibus. Taciti vel a etiam
curabitur velit torquent. Fusce interdum dictum vulputate sollicitudin
nulla. Orci placerat congue odio aptent enim mauris. Turpis nec
rhoncus eleifend eleifend eget. Auctor sed nullam vestibulum quisque
egestas; nullam aenean ante.
</p>
</details>
),
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ export function RecordTable_Sequences(

if (filteredRows == null) return null;

const warningText =
const errorText =
numSequences >= MIN_SEQUENCES_FOR_TREE &&
(filteredRows.length > MAX_SEQUENCES_FOR_TREE ||
filteredRows.length < MIN_SEQUENCES_FOR_TREE) ? (
Expand All @@ -663,7 +663,7 @@ export function RecordTable_Sequences(
} as CSSProperties
}
>
{warningText && <NoteBox type="error">{warningText}</NoteBox>}
{errorText && <NoteBox type="error">{errorText}</NoteBox>}
<div
style={{
padding: '10px',
Expand Down
Loading