ryan/feat(ux: create input loading spinner) #399

ryandotfurrer · 2024-07-13T16:21:31Z

input-loading-spinner.mp4

I created the LoadingSpinner component which is designed to be used in buttons when sending data to the database. It has a min-width/max-width and min-height/max-height equal to tailwind's w-7/w-9 and h-7/h-9 so it fits within the smallest and largest size variants of the Button component.

When a user clicks a submit button, the text should be replaced by a spinner
When a user clicks a submit button, the button should be disabled from being clicked again

Closes #91

…mponent optional props expect strings (tailwind classes) so the loader takes up the same amount of space as the element it is replacing before it loads in.

vercel · 2024-07-13T16:21:34Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
gridiron-survivor	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 20, 2024 0:28am
gridiron-survivor-storybook	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 20, 2024 0:28am

appwrite · 2024-07-13T16:21:34Z

Gridiron Survivor Application 6616ea581ef9f5521c7d

Function	ID	Status	Action
Your function has been successfully deployed.

Project name: Gridiron Survivor Application
Project ID: 6616ea581ef9f5521c7d

Function	ID	Status	Action
userAuth	`6626fef885a9f630442b`	Ready	View Logs

Only deployments on the production branch are activated automatically. If you'd like to activate this deployment, navigate to your deployments. Learn more about Appwrite Function deployments.

💡 Did you know?
You can use Avatars API to generate QR code for any text or URLs

…loading-spinner

…m/LetsGetTechnical/gridiron-survivor into ryan/create-input-loading-spinner

shashilo · 2024-07-19T15:13:35Z

components/LeagueCard/LeagueCard.tsx

@@ -23,7 +23,7 @@ const LeagueCard = React.forwardRef<HTMLAnchorElement, ILeagueCardProps>(
      data-testid="LeagueCard"
      href={href}
      className={cn(
-        'LeagueCard flex max-h-32 place-items-center gap-6 rounded-lg border bg-card p-4 text-card-foreground shadow-sm dark:border-zinc-800 hover:bg-zinc-800 transition',
+        'LeagueCard flex max-h-32 h-32 place-items-center gap-6 rounded-lg border bg-card p-4 text-card-foreground shadow-sm dark:border-zinc-800',


DO NOT set a hard height on this component. This will remove its flexibility and create bugs when content overflows.

@shashilo This is outdated, can it be resolved?

@shashilo pinging you for a reminder, please advise.

shashilo · 2024-07-19T15:14:21Z

app/(main)/league/all/page.tsx

-            <p className="text-lg font-bold">
-              You are not enrolled in any leagues
-            </p>
+            <LoadingSpinner height="max-h-32 h-32" />


Why is this max and hard set height?

I added props to the loader so you can set the height of it to match the element it's replacing. I'll revise this so it takes up 100% of the height and width of what it's replacing by default

@shashilo This is outdated, can it be resolved?

@shashilo pinging you for a reminder, please advise.

shashilo · 2024-07-19T15:15:24Z

app/(main)/league/all/page.tsx

@@ -56,9 +58,7 @@ const Leagues = (): JSX.Element => {
          ))
        ) : (
          <div className="text-center">
-            <p className="text-lg font-bold">


This is incorrect. The loader should replace to submit content as the API is waiting for a response. This message is correct when there are 0 leagues associated with the user.

Removed from this page

You reverted the
back, but the styles were not carried over.

@shashilo what loader is meant to be used on the LeagueCard component? I thought this input spinner was for buttons only?

@ryandotfurrer Was this question addressed?

<p className="text-lg font-bold"> These classes were removed. Wondering if this is intentional or not.

@ryandotfurrer Was this question addressed?

It was not. @shashilo, what loader is meant to be used on the LeagueCard component? I thought this input spinner was for buttons only?

<p className="text-lg font-bold"> These classes were removed. Wondering if this is intentional or not.

These are back in and will be reflected upon the next commit.

@ryandotfurrer Was this question addressed?

It was not. @shashilo, what loader is meant to be used on the LeagueCard component? I thought this input spinner was for buttons only?

@shashilo pinging you for a reminder, please advise.

app/(main)/login/page.tsx

vmaineng · 2024-08-29T19:36:50Z

app/(main)/register/page.tsx

@@ -115,6 +119,7 @@ const Register = (): JSX.Element => {
        />,
      );
    } catch (error) {
+      setIsLoading(false);


Could you please explain why setIsLoading(false) is used twice? One in the try part and in the catch.

Could you please explain why setIsLoading(false) is used twice? One in the try part and in the catch.

@vmaineng this was from @Clue355's branch of this branch that was approved and merged in.

Though I'd like to make this as similar to my code on the login page as possible.

@shashilo @Clue355 @vmaineng @chris-nowicki thoughts on me changing the onSubmit on the register/page.tsx to be inline with the one on the login/page.tsx?

Current Login onSubmit

const onSubmit: SubmitHandler<LoginUserSchemaType> = async ( data: LoginUserSchemaType, ): Promise<void> => { try { setIsLoading(true); await login(data); } catch (error) { console.error(error); throw new Error('An error occurred while logging in'); } finally { setIsLoading(false); } };

Current Register onSubmit

const onSubmit: SubmitHandler<RegisterUserSchemaType> = async ( data: RegisterUserSchemaType, ): Promise<void> => { try { setIsLoading(true); await registerAccount(data); await login(data); setIsLoading(false); router.push('/league/all'); toast.custom( <Alert variant={AlertVariants.Success} message="You have successfully registered your account." />, ); } catch (error) { setIsLoading(false); console.error('Registration Failed', error); toast.custom( <Alert variant={AlertVariants.Error} message="Something went wrong!" />, ); } };

@shashilo @Clue355 @vmaineng @chris-nowicki Please advise on the comment above.

@ryandotfurrer in what ways to you plan to make it inline? by adding a finally to the register try/catch?

@ryandotfurrer in what ways to you plan to make it inline? by adding a finally to the register try/catch?

@chris-nowicki precisely. Just to make them consistent. @vmaineng

Use finally. It allows you to only call the setIsLoading false once.

Use finally. It allows you to only call the setIsLoading false once.

@shashilo added in the latest commit

choir241 · 2024-09-16T17:31:35Z

app/(main)/login/page.test.tsx

  beforeEach(() => {
    jest.clearAllMocks();
+    jest.spyOn(React, 'useState').mockImplementation(useStateMock);


Does this not return an error in your local machine? I tried using this syntax for my loadingSpinner test, but it kept returning an error for me.

@choir27 it does; and it did before. I just haven't been able to get back to this one :P

@choir27 fixed

choir241 · 2024-09-16T17:32:39Z

app/(main)/login/page.test.tsx

+  it('should show the loading spinner', async () => {
+    (useStateMock as jest.Mock).mockImplementation((init: boolean) => [
+      true,
+      setIsLoading,
+    ]);


Your tests seem to be failing, please look into this and make sure it's changing the loading state properly

@choir27 I was aware :P Just been busy working on other things and unable to get back to this. TY!

@choir27 fixed

ryandotfurrer added 3 commits July 5, 2024 12:29

feat(LoadingSpinner.tsx): created first iteration of a loading spinner

fe61aee

feat(LoadingSpinner.tsx): added optional height and width props to co…

5d7fb40

…mponent optional props expect strings (tailwind classes) so the loader takes up the same amount of space as the element it is replacing before it loads in.

feat(LoadingSpinner.tsx): add accessibility features

86c9e36

ryandotfurrer requested review from shashilo and mhchen as code owners July 13, 2024 16:21

ryandotfurrer linked an issue Jul 13, 2024 that may be closed by this pull request

UX: Create input loading spinner #91

Closed

2 tasks

Merge branch 'develop' into ryan/create-input-loading-spinner

f69924e

vercel bot had a problem deploying to Preview – gridiron-survivor July 13, 2024 19:53 Failure

vercel bot deployed to Preview – gridiron-survivor-storybook July 13, 2024 19:54 View deployment

Merge branch 'develop' into ryan/create-input-loading-spinner

ba83fee

vercel bot deployed to Preview – gridiron-survivor-storybook July 16, 2024 01:15 View deployment

vercel bot had a problem deploying to Preview – gridiron-survivor July 16, 2024 01:15 Failure

Merge branch 'develop' into ryan/create-input-loading-spinner

879e45d

vercel bot deployed to Preview – gridiron-survivor-storybook July 16, 2024 22:08 View deployment

vercel bot had a problem deploying to Preview – gridiron-survivor July 16, 2024 22:09 Failure

Merge branch 'develop' into ryan/create-input-loading-spinner

0642af5

ryandotfurrer marked this pull request as draft July 17, 2024 00:14

ryandotfurrer added 2 commits July 17, 2024 16:29

Merge remote-tracking branch 'origin/develop' into ryan/create-input-…

9fe6c06

…loading-spinner

Merge branch 'ryan/create-input-loading-spinner' of https://github.co…

cfbb118

…m/LetsGetTechnical/gridiron-survivor into ryan/create-input-loading-spinner

vercel bot had a problem deploying to Preview – gridiron-survivor July 17, 2024 20:47 Failure

vercel bot deployed to Preview – gridiron-survivor-storybook July 17, 2024 20:48 View deployment

Merge branch 'develop' into ryan/create-input-loading-spinner

91a1cad

shashilo requested changes Jul 19, 2024

View reviewed changes

vercel bot deployed to Preview – gridiron-survivor-storybook July 19, 2024 15:17 View deployment

vercel bot had a problem deploying to Preview – gridiron-survivor July 19, 2024 15:17 Failure

Merge branch 'develop' into ryan/create-input-loading-spinner

3ae4268

vercel bot had a problem deploying to Preview – gridiron-survivor July 19, 2024 19:21 Failure

vercel bot deployed to Preview – gridiron-survivor-storybook July 19, 2024 19:22 View deployment

chris-nowicki requested changes Aug 29, 2024

View reviewed changes

app/(main)/login/page.tsx Outdated Show resolved Hide resolved

vmaineng reviewed Aug 29, 2024

View reviewed changes

remove redundant setIsLoading(false) from try statement

68f5ac5

vercel bot deployed to Preview – gridiron-survivor-storybook August 30, 2024 13:38 View deployment

vercel bot deployed to Preview – gridiron-survivor August 30, 2024 13:39 View deployment

Merge branch 'develop' into ryan/create-input-loading-spinner

edf7ccb

vercel bot deployed to Preview – gridiron-survivor September 5, 2024 01:44 View deployment

vercel bot deployed to Preview – gridiron-survivor-storybook September 5, 2024 01:45 View deployment

add finally statement to register page onSubmit function

585c807

vercel bot deployed to Preview – gridiron-survivor-storybook September 5, 2024 01:56 View deployment

vercel bot deployed to Preview – gridiron-survivor September 5, 2024 01:57 View deployment

Merge branch 'develop' into ryan/create-input-loading-spinner

564e8c4

vercel bot deployed to Preview – gridiron-survivor-storybook September 5, 2024 16:09 View deployment

vercel bot deployed to Preview – gridiron-survivor September 5, 2024 16:10 View deployment

choir241 requested changes Sep 16, 2024

View reviewed changes

Merge branch 'develop' into ryan/create-input-loading-spinner

cd6711d

vercel bot deployed to Preview – gridiron-survivor-storybook September 19, 2024 19:53 View deployment

vercel bot deployed to Preview – gridiron-survivor September 19, 2024 19:55 View deployment

update login/page.test.tsx tests

6beef05

vercel bot deployed to Preview – gridiron-survivor-storybook September 19, 2024 23:54 View deployment

vercel bot deployed to Preview – gridiron-survivor September 19, 2024 23:55 View deployment

fix register page tests

a81040b

vercel bot deployed to Preview – gridiron-survivor-storybook September 20, 2024 00:26 View deployment

vercel bot deployed to Preview – gridiron-survivor September 20, 2024 00:27 View deployment

Merge branch 'develop' into ryan/create-input-loading-spinner

f910e60

vercel bot deployed to Preview – gridiron-survivor-storybook September 20, 2024 00:28 View deployment

vercel bot deployed to Preview – gridiron-survivor September 20, 2024 00:28 View deployment

shashilo approved these changes Sep 20, 2024

View reviewed changes

shashilo merged commit 36b4ab6 into develop Sep 20, 2024
5 checks passed

shashilo deleted the ryan/create-input-loading-spinner branch September 20, 2024 14:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ryan/feat(ux: create input loading spinner) #399

ryan/feat(ux: create input loading spinner) #399

ryandotfurrer commented Jul 13, 2024 •

edited

Loading

vercel bot commented Jul 13, 2024 •

edited

Loading

appwrite bot commented Jul 13, 2024 •

edited

Loading

shashilo Jul 19, 2024

ryandotfurrer Aug 13, 2024

ryandotfurrer Sep 3, 2024

shashilo Jul 19, 2024

ryandotfurrer Jul 19, 2024

ryandotfurrer Aug 13, 2024

ryandotfurrer Sep 3, 2024

shashilo Jul 19, 2024

ryandotfurrer Jul 19, 2024

shashilo Jul 25, 2024

ryandotfurrer Jul 27, 2024

choir241 Aug 12, 2024

shashilo Aug 12, 2024

ryandotfurrer Aug 13, 2024

ryandotfurrer Aug 13, 2024

ryandotfurrer Sep 3, 2024

vmaineng Aug 29, 2024

ryandotfurrer Aug 30, 2024

ryandotfurrer Aug 30, 2024

ryandotfurrer Sep 3, 2024

chris-nowicki Sep 3, 2024

ryandotfurrer Sep 4, 2024 •

edited

Loading

shashilo Sep 5, 2024

ryandotfurrer Sep 5, 2024

choir241 Sep 16, 2024

ryandotfurrer Sep 19, 2024

ryandotfurrer Sep 20, 2024

choir241 Sep 16, 2024

ryandotfurrer Sep 19, 2024

ryandotfurrer Sep 20, 2024

ryan/feat(ux: create input loading spinner) #399

ryan/feat(ux: create input loading spinner) #399

Conversation

ryandotfurrer commented Jul 13, 2024 • edited Loading

vercel bot commented Jul 13, 2024 • edited Loading

appwrite bot commented Jul 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryandotfurrer Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryandotfurrer commented Jul 13, 2024 •

edited

Loading

vercel bot commented Jul 13, 2024 •

edited

Loading

appwrite bot commented Jul 13, 2024 •

edited

Loading

ryandotfurrer Sep 4, 2024 •

edited

Loading