Skip to content
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,4 @@ storybook-static/
!/e2e_testing/.env

e2e_testing/.yarn/cache
.swc
1 change: 1 addition & 0 deletions frontends/mit-learn/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"@storybook/react-webpack5": "^8.0.9",
"@storybook/test": "^8.0.9",
"@swc/core": "^1.4.11",
"@swc/plugin-emotion": "^4.0.0",
"@testing-library/react": "14.3.1",
"@testing-library/user-event": "14.5.2",
"@types/lodash": "^4.14.182",
Expand Down
104 changes: 49 additions & 55 deletions frontends/mit-learn/src/page-components/Dialogs/AddToListDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React, { useCallback, useId } from "react"
import React, { useCallback } from "react"
import {
Dialog,
LoadingSpinner,
Typography,
styled,
CheckboxChoiceField,
Button,
FormDialog,
DialogActions,
} from "ol-components"

import { RiAddLine } from "@remixicon/react"
Expand Down Expand Up @@ -33,17 +34,9 @@ const ResourceTitle = styled.span({
fontStyle: "italic",
})

const CheckboxContainer = styled.div({
padding: "28px 0",
})

const ButtonContainer = styled.div({
const Actions = styled(DialogActions)({
display: "flex",
gap: "12px",
alignItems: "flex-start",
button: {
width: "50%",
},
"> *": { flex: 1 },
})

type AddToListDialogInnerProps = {
Expand Down Expand Up @@ -100,7 +93,6 @@ const AddToListDialogInner: React.FC<AddToListDialogInnerProps> = ({
: null,
)
.filter((value) => value !== null)
const formId = useId()
const formik = useFormik({
enableReinitialize: true,
validateOnChange: false,
Expand Down Expand Up @@ -130,59 +122,61 @@ const AddToListDialogInner: React.FC<AddToListDialogInnerProps> = ({
})

return (
<Dialog
<FormDialog
Copy link
Contributor

Choose a reason for hiding this comment

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

I had considered using FormDialog, but at first it didn't seem necessary since this is barely a form and there's only one input element. This makes sense though, it cleans things up and onReset fixes our issue with the selections persisting.

title={dialogTitle}
showFooter={false}
fullWidth
onReset={formik.resetForm}
onSubmit={formik.handleSubmit}
{...NiceModal.muiDialogV5(modal)}
actions={
<Actions>
<Button
variant="primary"
type="submit"
disabled={!formik.dirty || isSaving}
>
Save
</Button>
<Button
variant="secondary"
startIcon={<RiAddLine />}
disabled={isSaving}
onClick={handleCreate}
>
Create New List
</Button>
</Actions>
}
>
{isReady ? (
<form id={formId} onSubmit={formik.handleSubmit}>
<>
<Typography variant="button">
Adding <ResourceTitle>{resource?.title}</ResourceTitle>
</Typography>
<CheckboxContainer>
{listType === ListType.LearningPath ? (
<CheckboxChoiceField
name="learning_paths"
choices={listChoices}
values={formik.values.learning_paths}
onChange={formik.handleChange}
vertical
/>
) : null}
{listType === ListType.UserList ? (
<CheckboxChoiceField
name="user_lists"
choices={listChoices}
values={formik.values.user_lists}
onChange={formik.handleChange}
vertical
/>
) : null}
</CheckboxContainer>
<ButtonContainer>
<Button
variant="primary"
type="submit"
disabled={!formik.dirty || isSaving}
>
Save
</Button>
<Button
variant="secondary"
startIcon={<RiAddLine />}
disabled={isSaving}
onClick={handleCreate}
>
Create New List
</Button>
</ButtonContainer>
</form>

{listType === ListType.LearningPath ? (
<CheckboxChoiceField
name="learning_paths"
choices={listChoices}
values={formik.values.learning_paths}
onChange={formik.handleChange}
vertical
/>
) : null}
{listType === ListType.UserList ? (
<CheckboxChoiceField
name="user_lists"
choices={listChoices}
values={formik.values.user_lists}
onChange={formik.handleChange}
vertical
/>
) : null}
</>
) : (
<LoadingSpinner loading={!isReady} />
)}
</Dialog>
</FormDialog>
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,6 @@ const UpsertLearningPathDialog = NiceModal.create(
onSubmit={formik.handleSubmit}
confirmText="Save"
noValidate
footerContent={
mutation.isError &&
!formik.isSubmitting && (
<Alert severity="error">
There was a problem saving your list. Please try again later.
</Alert>
)
}
>
<TextField
required
Expand Down Expand Up @@ -191,6 +183,11 @@ const UpsertLearningPathDialog = NiceModal.create(
value={formik.values.published}
onChange={(e) => formik.setFieldValue(e.name, e.value)}
/>
{mutation.isError && !formik.isSubmitting && (
<Alert severity="error">
There was a problem saving your list. Please try again later.
</Alert>
)}
Comment on lines +186 to +190
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's what it looks like now. Same as before.

Screenshot 2024-08-29 at 1 52 42 PM

</FormDialog>
)
},
Expand Down Expand Up @@ -242,14 +239,6 @@ const UpsertUserListDialog = NiceModal.create(
onSubmit={formik.handleSubmit}
confirmText={userList ? "Update" : "Create"}
noValidate
footerContent={
mutation.isError &&
!formik.isSubmitting && (
<Alert severity="error">
There was a problem saving your list. Please try again later.
</Alert>
)
}
>
<TextField
required
Expand Down Expand Up @@ -277,6 +266,11 @@ const UpsertUserListDialog = NiceModal.create(
multiline
minRows={3}
/>
{mutation.isError && !formik.isSubmitting && (
<Alert severity="error">
There was a problem saving your list. Please try again later.
</Alert>
)}
{userList && (
<div>
<Button
Expand Down
11 changes: 10 additions & 1 deletion frontends/mit-learn/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,16 @@ module.exports = (env, argv) => {
},
{
test: /\.tsx?$/,
use: "swc-loader",
use: {
loader: "swc-loader",
options: {
jsc: {
experimental: {
plugins: [["@swc/plugin-emotion", {}]],
},
},
},
},
Comment on lines +185 to +194
Copy link
Contributor Author

@ChristopherChudzicki ChristopherChudzicki Aug 29, 2024

Choose a reason for hiding this comment

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

This is https://www.npmjs.com/package/@swc/plugin-emotion

In particular, in dev mode, this gives us more useful class names. (production is unchanged).

Added in this PR in the course of trying to figure out where a style was coming from.

exclude: /node_modules/,
},
{
Expand Down
19 changes: 10 additions & 9 deletions frontends/ol-components/src/components/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { theme } from "../ThemeProvider/ThemeProvider"
import { default as MuiDialog } from "@mui/material/Dialog"
import type { DialogProps as MuiDialogProps } from "@mui/material/Dialog"
import { Button, ActionButton } from "../Button/Button"
import DialogActions from "@mui/material/DialogActions"
import MuiDialogActions from "@mui/material/DialogActions"
import { RiCloseLine } from "@remixicon/react"
import Typography from "@mui/material/Typography"
import Slide from "@mui/material/Slide"
Expand All @@ -26,7 +26,7 @@ const Content = styled.div`
margin: 28px 28px 40px;
`

export const Actions = styled(DialogActions)`
const DialogActions = styled(MuiDialogActions)`
margin: 0 28px 28px;
padding: 0;
gap: 4px;
Expand Down Expand Up @@ -62,10 +62,9 @@ type DialogProps = {
* [Dialog Props](https://mui.com/material-ui/api/dialog/#props).
*/
fullWidth?: boolean
showFooter?: boolean
isSubmitting?: boolean
PaperProps?: MuiDialogProps["PaperProps"]

actions?: React.ReactNode
disableEnforceFocus?: MuiDialogProps["disableEnforceFocus"]
}

Expand All @@ -87,7 +86,7 @@ const Dialog: React.FC<DialogProps> = ({
confirmText = "Confirm",
fullWidth,
className,
showFooter = true,
actions,
isSubmitting = false,
PaperProps,
disableEnforceFocus,
Expand Down Expand Up @@ -135,8 +134,10 @@ const Dialog: React.FC<DialogProps> = ({
{message && <Typography variant="body1">{message}</Typography>}
{children}
</Content>
{showFooter && (
<Actions>
{actions ? (
actions
) : (
<DialogActions>
<Button variant="secondary" onClick={onClose}>
{cancelText}
</Button>
Expand All @@ -148,11 +149,11 @@ const Dialog: React.FC<DialogProps> = ({
>
{confirmText}
</Button>
</Actions>
</DialogActions>
)}
</MuiDialog>
)
}

export { Dialog }
export { Dialog, DialogActions }
export type { DialogProps }
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,5 @@ export const Simple: Story = {
args: {
title: "Form Title",
fullWidth: true,
footerContent: "Footer content",
},
}
20 changes: 5 additions & 15 deletions frontends/ol-components/src/components/FormDialog/FormDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useCallback, useEffect, useMemo, useState } from "react"
import styled from "@emotion/styled"
import { Dialog } from "../Dialog/Dialog"
import type { DialogProps } from "@mui/material/Dialog"
import type { DialogProps } from "../Dialog/Dialog"

const FormContent = styled.div`
display: flex;
Expand Down Expand Up @@ -50,20 +50,12 @@ interface FormDialogProps {
* The form content. These will be direct children of MUI's [DialogContent](https://mui.com/material-ui/api/dialog-content/)
*/
children?: React.ReactNode
/**
* Extra content below the cancel/submit buttons. This is useful, e.g., for
* displaying overall form error messages.
*/
footerContent?: React.ReactNode
actions?: DialogProps["actions"]
/**
* Class applied to the `<form />` element.
*/
formClassName?: string

/**
* MUI Dialog's [TransitionProps](https://mui.com/material-ui/api/dialog/#props)
*/
TransitionProps?: DialogProps["TransitionProps"]
Comment on lines -63 to -66
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should have been removed when we made the dialogs "Slide down from top".

/**
* If `true`, the dialog stretches to its `maxWidth`.
*
Expand Down Expand Up @@ -93,7 +85,7 @@ const FormDialog: React.FC<FormDialogProps> = ({
title,
noValidate,
children,
footerContent,
actions,
confirmText = "Submit",
cancelText = "Cancel",
className,
Expand Down Expand Up @@ -139,11 +131,9 @@ const FormDialog: React.FC<FormDialogProps> = ({
isSubmitting={isSubmitting}
className={className}
PaperProps={paperProps}
actions={actions}
>
<FormContent>
{children}
{footerContent}
Comment on lines -144 to -145
Copy link
Contributor Author

@ChristopherChudzicki ChristopherChudzicki Aug 29, 2024

Choose a reason for hiding this comment

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

Anything that footerContent was used for can just be passed as part of the children.

(In a previous design, it needed to be separate)

</FormContent>
<FormContent>{children}</FormContent>
</Dialog>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const Container = styled.div<{ fullWidth?: boolean }>(({ fullWidth }) => [
display: "inline-flex",
flexDirection: "column",
alignItems: "start",
"> *": {
"> *:not(:last-child)": {
marginBottom: "4px",
Comment on lines -38 to 39
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This rule is supposed to make the 4px gap shown below. Note that there is no gap below the last child.

Figma:

Screenshot 2024-08-29 at 1 28 06 PM

},
},
Expand Down
6 changes: 3 additions & 3 deletions frontends/ol-components/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ export type { ContainerProps } from "@mui/material/Container"

export { default as MuiDialog } from "@mui/material/Dialog"
export type { DialogProps as MuiDialogProps } from "@mui/material/Dialog"
export { default as DialogActions } from "@mui/material/DialogActions"
export { default as MuiDialogActions } from "@mui/material/DialogActions"
export type { DialogActionsProps } from "@mui/material/DialogActions"
export { default as DialogContent } from "@mui/material/DialogContent"
export { default as MuiDialogContent } from "@mui/material/DialogContent"
export type { DialogContentProps } from "@mui/material/DialogContent"
export { default as DialogTitle } from "@mui/material/DialogTitle"
export { default as MuiDialogTitle } from "@mui/material/DialogTitle"
export type { DialogTitleProps } from "@mui/material/DialogTitle"

export { default as Divider } from "@mui/material/Divider"
Expand Down
Loading