Issue 65 replace all font styling with global font classes #172

SeiKasahara · 2024-07-25T21:05:44Z

Change Summary

Reworked Signup page.
Created new components and tested the dummy data to ensure we can register.
Created boundary check part.
Created regex pattern to check user's input

Related Issue

#65 #134

If all works done, the issue mentioned above should be closed.

Change Form

Fill this up (NA if not available). If a certain criteria is not met, can you please give a reason.

The pull request title has an issue number
The change works by "Smoke testing" or quick testing
The change has tests
The change has documentation

Other Information

Small bug exists
Do not merge this branch until all the bugs and progress were finished.

Type issues
Some Regex doesn't work, (e.g. valid username check with regex ^[\w.@+-]+$)
Some logic needs to be clarified (e.g. if photo was uploaded, progressbar goes or not?)
Avataruploader needs further modification to fit penni design style (Optional ?)
I didn't use <Form/> because I notice that once I use it, some functions doesn't work, for example:

//function...
  const handleEmailMobileChange = (e: ChangeEvent<HTMLTextTargetElement>) => {
    const value = e.target.value;
    setEmailMobile(value);
    setButtonVariant(value ? "default" : "inactive");
    if (isValidEmail(value) || isValidMobile(value)) {
      setErrorMessage(null);
      setButtonVariant("default");
    } else {
      setErrorMessage(errorMessagePreset);
      setButtonVariant("inactive");
    }
  }; // it doesn't work, I don't know why

return
       <Form onSubmit={()=>{example}}>
          <SingleLineInput
            name="emailMobile"
            onChange={handleEmailMobileChange}
            required={true}
            value={emailMobile}
            label="Email or mobile"
            type="text"
          /><Form/>
{/*middle part....*/}
        <Button
          className="h-[56px] w-full px-4 pb-4"
          variant={buttonInputVariant}
          type="submit"
          disabled={buttonInputVariant === "inactive"}
        >
          Continue
        </Button>

Related issue

Resolve Replace all font styling with global font classes #65

…re_Button_Variant_and_Create_a_Button_Reference_Example_Page

…styling_with_global_font_classes

vercel · 2024-07-25T21:05:47Z

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

Name	Status	Preview	Comments	Updated (UTC)
penni	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 31, 2024 3:54pm

ErikaKK

Pls remove padding style in all buttons. Pls see my comments. Pls check the reason for your type check failure

ErikaKK · 2024-07-26T07:35:25Z

client/src/components/ui/signup/main-page.tsx

+          </div>
+        )}
+        <Button
+          className="h-[56px] w-full px-4 pb-4"


Pls remove padding. This makes the text not vertically center

Replaced all button to normal and proper style.

ErikaKK · 2024-07-26T07:36:42Z

client/src/components/ui/signup/main-page.tsx

+        {errorMessage === errorMessagePreset ? (
+          <div className="callout text-red-600">{errorMessage}</div>
+        ) : (
+          <div className="callout text-red-600">


Pls use ErrorCallout instead

ErikaKK · 2024-07-26T07:42:01Z

client/src/components/ui/signup/avatar.tsx

+            </button>
+          </div>
+
+          {showOptions && (


This showOptions should be set to false after upload img

ErikaKK · 2024-07-26T07:51:00Z

client/src/components/ui/signup/pw.tsx

+    setRepeatPassword(value);
+    setRepeatPasswordVariant(e.target.value ? "default" : "inactive");
+    if (passwordValidation(password, value)) {
+      // 修改此行


Should add password rule message and validation. Pls ask backend how they designed it

They didn't have a clear criteria. I need to learn it by myself.

ErikaKK · 2024-07-26T07:52:39Z

client/src/pages/signup.tsx

+          setTitle("Welcome!");
+          setClient("Poster");
+          console.log("Successfully created a new user");
+          window.location.href = "/index";


There is no /index page. You should direct them to /poster or /bidder based on their role. Pls use router.push() to redirect

ErikaKK · 2024-07-26T08:06:13Z

client/src/pages/signup.tsx

-  ];
-  const multistepForm: MultistepForm = useMultistepForm(forms);
+      try {
+        const response = await api.post("/app/register/", data);


In the sign up page, you should send two post requests. One is sending to "/app/register/", another one is sending to "/app/profiles/". @yunho7687 Is this how it works?

/app/profiles was destoryed. According to @yunho7687

SodaVolcano · 2024-07-26T12:46:23Z

client/src/components/ui/signup/avatar.tsx

+            <div className="fixed inset-0 z-50 flex items-end justify-center bg-black bg-opacity-50 transition-transform">
+              <div className="duration-800 animate-slide-up w-full space-y-1 rounded-t-lg p-4 ease-in-out">
+                <Button
+                  className="flex h-[56px] w-full px-4"


is that [...] I see???? don't use it pls :)

I'll ignore the rest to not clutter up the comments but pls change other occurrences as well

SodaVolcano · 2024-07-26T12:47:52Z

client/src/components/ui/signup/avatar.tsx

+          )}
+        </div>
+      </div>
+      <div className="absolute left-0 right-0 top-[574px] flex flex-col gap-3 p-[1px_0] px-4">


same for this, not sure why you would need such a large value as [574px], maybe a constant pixel value is not the right choice (i.e. look into using vw etc. For example, the sign in page used top padding that didn't use a specific pixel size. and what's [1px_0]???

SodaVolcano · 2024-07-26T12:53:15Z

client/src/components/ui/signup/check-unique.ts

+export const checkUnique = async (input: string): Promise<boolean> => {
+  try {
+    // this check logic should be revised, it will be a mess as data amount grows.
+    const response = await api.get("app/users/");
+    // axios automatically parses the response as JSON if the content type is application/json
+    const data = response.data;
+    for (const profile of data) {
+      if (profile.full_name === input) {
+        return true;
+      }
+    }
+    return false;
+  } catch (error) {
+    const axiosError = error as AxiosCustomError;
+    if (axiosError.response) {
+      // The request was made and the server responded with a status code
+      // that falls out of the range of 2xx
+      throw new Error(
+        `Server responded with status: ${axiosError.response.status}`,
+      );
+    } else if (axiosError.request) {
+      // The request was made but no response was received
+      throw new Error("No response received from server");
+    } else {
+      // Something happened in setting up the request that triggered an Error
+      throw new Error(`Error in setting up request: ${axiosError.message}`);
+    }
+  }
+};


What is this function used for? If it's to check if user name already exist in the database, that's the backend's job. use the API users/<user_id to check if server responded with the user info (meaning it exist in database). Don't think the API exists so make an issue and assign me and I'll add it to the backend.

Alternatively you can add it yourself, just need to edit url.py and views.py

That makes sense I can made it by myself

SodaVolcano · 2024-07-26T12:56:14Z

client/src/components/ui/signup/main-page.tsx

+      <div className="absolute left-0 right-0 top-[312px] flex h-[192px] w-full flex-col items-center justify-center gap-3 px-4">
+        <div className="w-full">
+          <SingleLineInput
+            name="emailMobile"
+            onChange={handleEmailMobileChange}
+            required={true}
+            value={emailMobile}
+            label="Email or mobile"
+            type="text"
+          />
+        </div>
+        {errorMessage === errorMessagePreset ? (
+          <div className="callout text-red-600">{errorMessage}</div>
+        ) : (
+          <div className="callout text-red-600">
+            {errorMessage}{" "}
+            {!(errorMessage === null) && (
+              <Link className="callout text-penni-main" href="/sign-in">
+                sign in
+              </Link>
+            )}
+          </div>
+        )}
+        <Button
+          className="h-[56px] w-full px-4 pb-4"
+          variant={buttonInputVariant}
+          onClick={handleSubmit}
+          disabled={buttonInputVariant === "inactive"}
+        >
+          Continue
+        </Button>
+        {client === "Poster" ? (
+          <Button
+            className="h-[56px] w-full px-4 pb-4"
+            variant="link"
+            onClick={() => setClient("Bidder")}
+          >
+            Switch to Bidder
+          </Button>
+        ) : (
+          <Button
+            className="h-[56px] w-full px-4 pb-4"
+            variant="link"
+            onClick={() => setClient("Poster")}
+          >
+            Switch to Poster
+          </Button>
+        )}
+      </div>
+    </div>
+  );
+};


For <Form>, I was stupid and had this line in all Input components inside input.tsx

onChange = context?.onChange ?? onChange ?? ((e) => {});

So, this is going to check if the context (the <Form> component) will supply the onChange method. If not, it falls back on the onChange in the prop. Since you wrapped it inside <Form> it'll always use the onChange provided by the form. To fix this, just change that line to

onChange = onChange ?? context?.onChange ?? ((e) => {});

and you should be able to override the onChange function inside <Form>

Also, consider moving validation for some inputs to the onSubmit function for the form. This is pretty much up to you whether you think it belongs in the onChange functions or in the onSubmit function, could make code more concise by letting <Form> manage the state (but the validation is only done when user clicks the button)

UPDATE: do the same with value if the logic for that is the same (i.e. instead of checking for context first, it should check for props.value first

SodaVolcano · 2024-07-26T12:57:20Z

client/src/components/ui/signup/pw.tsx

+  if (repeatPassword != password) {
+    return false;
+  }
+  return true;
+}


Change to return repeatedPassword == password

SodaVolcano · 2024-07-26T12:58:14Z

client/src/components/ui/signup/pw.tsx

+    setRepeatPassword(value);
+    setRepeatPasswordVariant(e.target.value ? "default" : "inactive");
+    if (passwordValidation(password, value)) {
+      // 修改此行


Remove Chinese comment, not everyone can read it :(

Edit: okay everyone working on penni right now probably can read it but avoid having such comments in future projects

haha bro just forgot to delete em lol

SodaVolcano · 2024-07-26T12:58:45Z

client/src/components/ui/signup/pw.tsx

+        <span className="body pd-4 px-4 text-center text-primary">
+          Please enter your password and confirm password
+        </span>
+        <div className="absolute left-0 right-0 top-[160px] flex h-[192px] flex-col items-center justify-center gap-3 px-4">
+          <div className="w-full">
+            <SingleLineInput
+              label="Password"
+              onChange={handlePasswordChange}
+              value={password}
+              required={true}
+              type="password"
+            />
+          </div>
+          <div className="w-full">
+            <SingleLineInput
+              label="Confirm Password"
+              onChange={handleRepeatPasswordChange}
+              value={repeatPassword}
+              required={true}
+              type="password"
+            />
+          </div>
+          {errorMessage && (
+            <div className="callout text-red-600">{errorMessage}</div>
+          )}
+          <Button
+            className="h-[56px] w-full px-4 pb-4"
+            variant={buttonVariant}
+            onClick={() => setCurrentStep(currentStep + 1)}
+            type="submit"
+            disabled={buttonVariant === "inactive"}
+          >
+            Continue
+          </Button>
+        </div>
+      </div>
+    </div>
+  );
+};


You should be able to use <Form> here with the suggested change in above comment.

SodaVolcano · 2024-07-26T13:05:52Z

client/src/components/ui/signup/regex.ts

+// regex.ts
+export const emailRegex = new RegExp(
+  "^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$",
+);
+export const mobileRegex = new RegExp("^[0-9]{10}$");
+
+export const userNameRegex = new RegExp("^[w.@+-]+$");


Why is there a file for just regex? Should be in the file that they are used in

I'm thinking, in the future, if people want to validate more forms, they can extract regular expressions from this file. I actually want to move this to another location instead of keeping it separately in the components/ui/signup folder.

Hm.. it doesn't make sense to separate this into its own file, regex strings are nothing more than just variables and we don't usually separate variables out into its own file (unless the variable is a global constant/configuration that affects the whole project)

…styling_with_global_font_classes

Changes to be committed: modified: client/src/components/ui/callout.tsx modified: client/src/components/ui/signup/avatar.tsx modified: client/src/components/ui/signup/bio.tsx modified: client/src/components/ui/signup/check-unique.ts modified: client/src/components/ui/signup/main-page.tsx modified: client/src/components/ui/signup/pw.tsx modified: client/src/components/ui/signup/regex.ts modified: client/src/components/ui/signup/welcome.tsx modified: client/src/pages/signup.tsx modified: server/app/urls.py modified: server/app/views.py

SeiKasahara · 2024-07-29T18:25:11Z

Enhancement Log

Designed a pop-up animation for the error callout on the signup page without affecting the original layout.
The issues with types and regular expressions have been resolved.
Using app/register for email uniqueness validation caused users to be registered directly during the validation phase, resulting in a 400 status code on subsequent registration attempts. Therefore, a custom validate API was created to allow a separate GET API for email uniqueness validation, with URL injection prevention. A separate POST API is used to prevent password brute-force attacks (by using the common password validation).
Added a token to local storage for users to directly access the corresponding page after registration.
The avatar can now be captured using the camera.
Modified most of the TailwindCSS styles, using h-number instead of h-[number px].
Added back and skip buttons on the progress bar for users to set content in the future.
Basically all anomaly incident will be prevent in the frontend part. they must pass the frontend validation to access the final submission.

ErikaKK

Just needs a bit change. Pls see my comments

ErikaKK · 2024-07-30T15:18:03Z

client/src/components/ui/signup/pw.tsx

+    inputCheck(value, setPasswordVariant);
+  };
+
+  const handleRepeatPasswordChange = (


This one doesn't check when changing the first password input field.

Needs to add to the first pw input field

ErikaKK · 2024-07-30T15:18:08Z

client/src/components/ui/signup/pw.tsx

+    password: string,
+    hook: React.Dispatch<React.SetStateAction<string>>,
+  ) {
+    if (password.length <= 8) {


change to password.length < 8

ErikaKK · 2024-07-30T15:20:45Z

client/src/components/ui/signup/pw.tsx

+    if (password.length <= 8) {
+      setErrorMessage(
+        "This password is too short. It must contain at least 8 characters.",
+      );


Password rules should be more detailed. e.g. not too common

The common validation part will be validated in backend

ErikaKK · 2024-07-30T15:25:54Z

client/src/pages/signup.tsx

+        email: emailMobile,
+        username: fullName,
+        password: password,
+        //avatar: profilePhoto,


change to avatar_url and uncomment this

ErikaKK · 2024-07-30T15:29:47Z

client/src/components/ui/signup/main-page.tsx

+const uniqueErrorMessage = (
+  <>
+    Email or Mobile has been registered, please{" "}
+    <Link className="text-penni-main" href="/sign-in">


Pls change /sign-in to /login bc I changed the name of sign-in page

SodaVolcano

Yeah nah I'm not gonna review the rest, this project is near the end so I'm not gonna check the code that thoroughly. If you don't want to change the code you can dismiss this change request on GitHub

SodaVolcano · 2024-07-30T15:30:30Z

client/src/components/ui/callout.tsx

 import { EditIcon, InfoIcon } from "./icons";

 interface CalloutProps {
-  text: string;
+  text: React.ReactNode;


?????? why is this being changed???

If you want links in the callout, set onClick, this will probably break where ever Callout is used

errorMessage text can be null, can I change to string | null ?

no it shouldn't be nullable, was it modified somewhere?

I notice that if it’s empty string the error-callout component still takes position, but if the text is null it won’t.Since user input is correct, the error-callout should be hided. So I tried to set error messages as <string | null>

If the error callout should be hidden then you shouldn't be rendering it at all. Don't hide it by messing with the string parameter, hide it by not rendering it. Other pages kept a state errorOccured and if it's set to true then the error callout is rendered

SodaVolcano · 2024-07-30T15:31:48Z

client/src/components/ui/signup/avatar.tsx

          Upload a profile photo so others can recognise you!
        </span>
-        {/*Avatar Uploader*/}
-        <div className="flex items-center justify-center">
+        <div className="mt-2 flex">
          <div className="relative h-[136px] w-[136px]">


h-[136px] w-[136px] change

SodaVolcano · 2024-07-30T15:32:06Z

client/src/components/ui/signup/avatar.tsx

+              <div className="fixed inset-0 z-50 flex flex-col items-center justify-center bg-black bg-opacity-70 transition-transform">
+                <div className="mt-56 flex flex-col items-center justify-center">
+                  <Webcam
+                    className="h-[136px] w-[136px] rounded-full border-2 object-cover"


h-[136px] w-[136px]

SodaVolcano · 2024-07-30T15:32:48Z

client/src/components/ui/signup/check-unique.ts

-      if (profile.full_name === input) {
-        return true;
-      }
+    // console.log(data);


remove if you are commenting this out

ErikaKK · 2024-07-30T15:45:41Z

FYI, This is the new fields (plus avatar_url) that you have to submit when adding a user

SeiKasahara · 2024-07-30T15:57:51Z

FYI, This is the new fields (plus avatar_url) that you have to submit when adding a user

is that avatar data requirement new added?

…styling_with_global_font_classes

yunho7687 · 2024-07-31T14:39:58Z

@SodaVolcano thanks for all your dedication! 🐐 you are the real one!

yunho7687 · 2024-07-31T14:40:55Z

@ErikaKK @SeiKasahara thanks for your dedication!

SodaVolcano · 2024-07-31T15:43:42Z

@SodaVolcano thanks for all your dedication! 🐐 you are the real one!

@ErikaKK should get the credit since she helped out waaaaay more than me HAHA 🫡🫡🫡🫡🫡🎉🎉🎉 absolute commitment!!

SeiKasahara · 2024-07-31T15:45:43Z

@ErikaKK @SeiKasahara thanks for your dedication!

Great Gig too!

…ling

yunho7687 · 2024-07-31T16:30:19Z

several convention issues but run out of time :(

SeiKasahara added 5 commits July 20, 2024 04:35

store all works

01fc457

Merge remote-tracking branch 'origin' into issue-70-Request_to_Add_Mo…

979bf76

…re_Button_Variant_and_Create_a_Button_Reference_Example_Page

Merge remote-tracking branch 'origin' into issue-65-Replace_all_font_…

f6b23c2

…styling_with_global_font_classes

reworked the main sign-up page

089fb12

finished basic functions to test

8b13a8e

SeiKasahara added enhancement New feature or request frontend Task must have a front end issue priority:medium difficulty:hard labels Jul 25, 2024

SeiKasahara requested a review from ArcueidShiki July 25, 2024 21:05

SeiKasahara self-assigned this Jul 25, 2024

SeiKasahara requested review from ErikaKK and SodaVolcano July 26, 2024 03:35

ErikaKK requested changes Jul 26, 2024

View reviewed changes

SodaVolcano requested changes Jul 26, 2024

View reviewed changes

SeiKasahara added 2 commits July 29, 2024 06:29

Merge remote-tracking branch 'origin' into issue-65-Replace_all_font_…

7b9c607

…styling_with_global_font_classes

SeiKasahara requested a review from yunho7687 July 29, 2024 18:15

vercel bot deployed to Preview July 29, 2024 18:15 View deployment

ErikaKK requested changes Jul 30, 2024

View reviewed changes

SodaVolcano requested changes Jul 30, 2024

View reviewed changes

finished all required field

19b4588

vercel bot deployed to Preview July 30, 2024 16:17 View deployment

Merge remote-tracking branch 'origin' into issue-65-Replace_all_font_…

1f09ca5

…styling_with_global_font_classes

vercel bot deployed to Preview July 31, 2024 10:05 View deployment

chore: Refactor code to remove unnecessary lines and improve readability

140153a

vercel bot deployed to Preview July 31, 2024 14:35 View deployment

Refactor Callout component to require non-null text prop

2346043

Refactor signup components to improve code readability and error hand…

9209f75

…ling

vercel bot deployed to Preview July 31, 2024 15:54 View deployment

yunho7687 merged commit f6eccfe into main Jul 31, 2024
7 checks passed

yunho7687 deleted the issue-65-Replace_all_font_styling_with_global_font_classes branch July 31, 2024 16:30

Issue 65 replace all font styling with global font classes #172

Issue 65 replace all font styling with global font classes #172

Conversation

SeiKasahara commented Jul 25, 2024 • edited Loading

Change Summary

Related Issue

Change Form

Other Information

Related issue

vercel bot commented Jul 25, 2024 • edited Loading

ErikaKK left a comment • 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

SeiKasahara Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SodaVolcano Jul 26, 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

SeiKasahara commented Jul 29, 2024 • edited Loading

Enhancement Log

ErikaKK left a 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

SodaVolcano left a 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

ErikaKK commented Jul 30, 2024 • edited Loading

SeiKasahara commented Jul 30, 2024

yunho7687 commented Jul 31, 2024

yunho7687 commented Jul 31, 2024

SodaVolcano commented Jul 31, 2024

SeiKasahara commented Jul 31, 2024

yunho7687 commented Jul 31, 2024

SeiKasahara commented Jul 25, 2024 •

edited

Loading

vercel bot commented Jul 25, 2024 •

edited

Loading

ErikaKK left a comment •

edited

Loading

SeiKasahara Jul 26, 2024 •

edited

Loading

SodaVolcano Jul 26, 2024 •

edited

Loading

SeiKasahara commented Jul 29, 2024 •

edited

Loading

ErikaKK commented Jul 30, 2024 •

edited

Loading