Skip to content
This repository has been archived by the owner on Apr 10, 2023. It is now read-only.

Added visual cueing when submitting forms #52

Merged
merged 3 commits into from
Jan 29, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 5 additions & 3 deletions src/app/Layout.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { useState } from 'react';
import { Navigate, Outlet } from 'react-router-dom';
import SuccessNotification from '~/components/SuccessNotification';
import { useSession } from '~/data/hooks/session';
import ErrorNotification from '../components/ErrorNotification';
import { ErrorProvider } from '../components/ErrorProvider';
import Footer from '../components/Footer';
import Header from '../components/Header';
import { NotificationProvider } from '../components/NotificationProvider';
import Sidebar from '../components/Sidebar';

function InnerLayout() {
Expand Down Expand Up @@ -34,9 +35,10 @@ function InnerLayout() {

export default function Layout() {
return (
<ErrorProvider>
<NotificationProvider>
<InnerLayout />
<ErrorNotification />
</ErrorProvider>
<SuccessNotification />
</NotificationProvider>
);
}
23 changes: 0 additions & 23 deletions src/components/ErrorProvider.tsx

This file was deleted.

21 changes: 18 additions & 3 deletions src/components/Loading.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,22 @@
export default function Loading() {
type LoadingProps = {
isButton?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think this is actually used anywhere else, the Loading component, so we could probably just make this only apply to buttons

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I wasn't sure if this was planned to be use elsewhere. Removed the isButton logic/prop

isPrimary?: boolean;
};

export default function Loading(props: LoadingProps) {
const { isButton, isPrimary } = props;

return (
<div className="flex h-screen items-center justify-center">
<div className="h-10 w-10 animate-spin rounded-full border-b-2 border-violet-300"></div>
<div
className={`flex ${
!isButton ? 'h-screen' : ''
} items-center justify-center`}
>
<div
className={`${isButton ? 'h-5 w-5' : 'h-10 w-10'} ${
isPrimary ? 'border-white-300' : 'border-violet-300'
} animate-spin rounded-full border-b-2`}
></div>
</div>
);
}
48 changes: 48 additions & 0 deletions src/components/NotificationProvider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { createContext, useState } from 'react';

type ErrorContext = {
error: Error | null;
setError(_error: Error | null): void;
clearError(): void;
};

type SuccessContext = {
success: string | null;
setSuccess(_success: string | null): void;
clearSuccess(): void;
};

export const NotificationContext = createContext<ErrorContext & SuccessContext>(
{
error: null as Error | null,
setError(_error: Error | null) {},
clearError() {},
success: null as string | null,
setSuccess(_success: string | null) {},
clearSuccess() {}
}
);

export function NotificationProvider({
children
}: {
children: React.ReactNode;
}) {
const [error, setError] = useState<Error | null>(null);
const [success, setSuccess] = useState<string | null>(null);

return (
<NotificationContext.Provider
value={{
error,
setError,
clearError: () => setError(null),
success,
setSuccess,
clearSuccess: () => setSuccess(null)
}}
>
{children}
</NotificationContext.Provider>
);
}
39 changes: 39 additions & 0 deletions src/components/SuccessNotification.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { Transition } from '@headlessui/react';
import { CheckCircleIcon, XMarkIcon } from '@heroicons/react/20/solid';
import { useSuccess } from '~/data/hooks/success';

export default function SuccessNotification() {
const { success, setSuccess } = useSuccess();

return (
<Transition show={success !== null}>
Copy link
Contributor

Choose a reason for hiding this comment

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

could we make the success notification clear itself after a short period? say 2-3 seconds? i think success notifications should be short-lived, while error notifications should persist until manually closed/acknowledged. wyt?

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 is a good thought, I wasn't sure if this was a desired functionality. Set timeout to be 3 seconds for now.

<div className="max-w-s fixed bottom-0 right-2 z-10 m-4">
<div className="rounded-md bg-green-50 p-4">
<div className="flex">
<div className="flex-shrink-0">
<CheckCircleIcon
className="h-5 w-5 text-green-400"
aria-hidden="true"
/>
</div>
<div className="ml-3">
<p className="text-sm font-medium text-green-800">{success}</p>
</div>
<div className="ml-auto pl-3">
<div className="-mx-1.5 -my-1.5">
<button
type="button"
onClick={() => setSuccess(null)}
className="inline-flex rounded-md bg-green-50 p-1.5 text-green-500 hover:bg-green-100 focus:outline-none focus:ring-2 focus:ring-green-600 focus:ring-offset-2 focus:ring-offset-green-50"
>
<span className="sr-only">Dismiss</span>
<XMarkIcon className="h-5 w-5" aria-hidden="true" />
</button>
</div>
</div>
</div>
</div>
</div>
</Transition>
);
}
21 changes: 17 additions & 4 deletions src/components/flags/FlagForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import Input from '~/components/forms/Input';
import Toggle from '~/components/forms/Toggle';
import { createFlag, updateFlag } from '~/data/api';
import { useError } from '~/data/hooks/error';
import { useSuccess } from '~/data/hooks/success';
import { keyValidation, requiredValidation } from '~/data/validations';
import { IFlag, IFlagBase } from '~/types/Flag';
import { stringAsKey } from '~/utils/helpers';
import Loading from '../Loading';

type FlagFormProps = {
flag?: IFlag;
Expand All @@ -18,8 +20,10 @@ type FlagFormProps = {
export default function FlagForm(props: FlagFormProps) {
const { flag, flagChanged } = props;
const isNew = flag === undefined;
const submitPhrase = isNew ? 'Create' : 'Update';
const navigate = useNavigate();
const { setError, clearError } = useError();
const { setSuccess } = useSuccess();

const handleSubmit = (values: IFlagBase) => {
if (isNew) {
Expand All @@ -43,7 +47,9 @@ export default function FlagForm(props: FlagFormProps) {
handleSubmit(values)
.then(() => {
clearError();

setSuccess(
`Successfully ${submitPhrase.toLocaleLowerCase()}d flag`
);
if (isNew) {
navigate(`/flags/${values.key}`);
return;
Expand All @@ -52,6 +58,7 @@ export default function FlagForm(props: FlagFormProps) {
flagChanged && flagChanged();
})
.catch((err) => {
console.log(err);
setError(err);
});
}}
Expand Down Expand Up @@ -147,11 +154,17 @@ export default function FlagForm(props: FlagFormProps) {
</Button>
<Button
primary
className="ml-3"
className="ml-3 min-w-[80px]"
type="submit"
disabled={!(formik.dirty && formik.isValid)}
disabled={
!(formik.dirty && formik.isValid && !formik.isSubmitting)
}
>
{isNew ? 'Create' : 'Update'}
{formik.isSubmitting ? (
<Loading isButton isPrimary />
) : (
submitPhrase
)}
</Button>
</div>
</div>
Expand Down
18 changes: 16 additions & 2 deletions src/components/flags/VariantForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import TextArea from '~/components/forms/TextArea';
import MoreInfo from '~/components/MoreInfo';
import { createVariant, updateVariant } from '~/data/api';
import { useError } from '~/data/hooks/error';
import { useSuccess } from '~/data/hooks/success';
import { jsonValidation, keyValidation } from '~/data/validations';
import { IVariant, IVariantBase } from '~/types/Variant';
import Loading from '../Loading';

type VariantFormProps = {
setOpen: (open: boolean) => void;
Expand All @@ -22,7 +24,9 @@ export default function VariantForm(props: VariantFormProps) {
const { setOpen, flagKey, variant, onSuccess } = props;
const isNew = variant === undefined;
const title = isNew ? 'New Variant' : 'Edit Variant';
const submitPhrase = isNew ? 'Create' : 'Update';
const { setError, clearError } = useError();
const { setSuccess } = useSuccess();

const handleSubmit = async (values: IVariantBase) => {
if (isNew) {
Expand All @@ -44,6 +48,9 @@ export default function VariantForm(props: VariantFormProps) {
handleSubmit(values)
.then(() => {
clearError();
setSuccess(
`Successfully ${submitPhrase.toLocaleLowerCase()}d variant.`
Copy link
Contributor

Choose a reason for hiding this comment

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

in some cases we end this message with a . and others we don't, like: https://github.com/flipt-io/flipt-ui/pull/52/files#diff-6fdc396f20dbeed901b28539b86f9ee4c7097a9f8aae89882a0c374dddfbb099R51

We should probably be consistent in the messaging/punctation, I'm just not sure which one I prefer (with the period or without). Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I actually like without the .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the . from success messages

);
onSuccess();
})
.catch((err) => {
Expand Down Expand Up @@ -155,10 +162,17 @@ export default function VariantForm(props: VariantFormProps) {
<Button onClick={() => setOpen(false)}>Cancel</Button>
<Button
primary
className="min-w-[80px]"
type="submit"
disabled={!(formik.dirty && formik.isValid)}
disabled={
!(formik.dirty && formik.isValid && !formik.isSubmitting)
}
>
{isNew ? 'Create' : 'Update'}
{formik.isSubmitting ? (
<Loading isButton isPrimary />
) : (
submitPhrase
)}
</Button>
</div>
</div>
Expand Down
Loading