-
Notifications
You must be signed in to change notification settings - Fork 3
Create Kas Submission Page #16
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
base: development
Are you sure you want to change the base?
Create Kas Submission Page #16
Conversation
|
@jeflijonathan2327240094 |
c937c5e to
ba48f60
Compare
fanesz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Untuk sementara ini dulu, baru aku review dari segi penulisan kode, untuk UI dan fungsionalitas itu nanti.
Rombak folder structure, samain polanya kayak yg admin - financial request.
jadi harusnya pages/KasSubmission, baru didalemnya ada context, hooks, partials.
trus karena ada fitur spesifik, bikin folder List yg isinya komponen dan hooks untuk nampilin list. Trus folder Create. intinya pahamin dan ikutin aja pattern yg procom-admin
kalo ada pertanyaan/bingung/kendala/error, feel free pm aku
package.json
Outdated
| "react-loading-skeleton": "^3.4.0", | ||
| "react-router-dom": "^6.23.1", | ||
| "sweetalert2": "^11.12.3", | ||
| "sweetalert2-react-content": "^5.0.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buang 2 package ini kalo cuma untuk error notifikasi, pake snackbar yg sudah di develop disini
src/api/index.ts
Outdated
| Accept: "application/json", | ||
| "Content-type": "application/json", | ||
| }; | ||
| headers: Headers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
konsep headers ini di rombak lagi, untuk default headers, ttp pake application/json,
trus bikin 1 method baru, POSTFORM (referensi), nah method ini ngeassign manual headers dengan tipe multipart/form-data, dll.
src/api/index.ts
Outdated
| } catch (err: AxiosError | any) { | ||
| if (isAxiosError(err)) { | ||
| return err?.response?.data; | ||
| console.error("Axios error:", err.message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buang console log dan commend di file ini
src/api/services/CreateKas.ts
Outdated
| evidence: string; | ||
| }; | ||
|
|
||
| export default class CreateKasService { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cek lagi naming patternnya, harusnya ini KasSubmissionService, file pagesnya juga jangan spesifik di CreateKas, cek lagi di admin repo yang financial request, disitu ada folder FinancialRequest/Create, disitu baru taro logic dan component untuk create
src/api/services/CreateKas.ts
Outdated
| userPath: string = "/users"; | ||
| private api: API = new API(); | ||
| private apiForm: API = new API({ isForm: true }); | ||
| async post( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tolong newline spacing di perhatiin, cek lagi yg admin, dimana mana harus pake breakpoint newline
| {loading ? ( | ||
| <div>Loading...</div> | ||
| ) : ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loading ini harusnya di parent kayak gini.
jadi nampilin loading dialog dulu, baru nampilin actual dialog with data
src/api/services/UploadImage.ts
Outdated
| export type UploadImageRequest = { | ||
| file: File; | ||
| }; | ||
| export default class UploadImage { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhatiin namanya, harusnya FileService
| import { useRef, useEffect } from "react"; | ||
| import { useCreateKasContext } from "@pages/CreateKas/context/index"; | ||
| import glassmorphism from "@utils/glassmorphism"; | ||
| const QuickNote = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note ini untuk apa? kok banyak event listener gini
| import UploadImage from "./UploadImage"; | ||
| import useCreateKasSubmission from "../hooks/useCreateKasSubmission"; | ||
| import SearchUser from "./SearchUser"; | ||
| const KasBody: React.FC = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
untuk seluruh file, perhatiin newline break, setelah import itu dikasih line break
…ions file add: subbmission form request and error required
|
|
src/common/utils/localStorage.ts
Outdated
| @@ -0,0 +1,18 @@ | |||
| export const LocalStorage = (key: string) => { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
penamaan pascal case itu biasanya untuk nama class atau component.
kalo cuma variable/function, pake camel case
| export const LocalStorage = (key: string) => { | |
| export const localStorage = (key: string) => { |
| import useGetUser from "../hooks/useGetUser"; | ||
| import { LocalStorage } from "@utils/localStorage"; | ||
|
|
||
| const SearchUser = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
penamaan komponen bikin lebih spesifik,
misal SearchUserButton
| import { useFormContext } from "react-hook-form"; | ||
| import { LocalStorage } from "@utils/localStorage"; | ||
|
|
||
| const QuickNote: React.FC = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
penamaan komponen bikin lebih spesifik
| import glassmorphism from "@utils/glassmorphism"; | ||
| import { Controller, useFormContext } from "react-hook-form"; | ||
|
|
||
| const PayedAmount: React.FC = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
penamaan komponen bikin lebih spesifik
| handleActiveUser: () => void; | ||
| } | ||
|
|
||
| const useGetUser = (): HookReturn => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
terlalu spesifik, kalo getUser lebih jadi kayak usecase, bukan hooks,
mending useUser. yg relate dgn user, taruh disini nanti
…opzone, and change name to useLocalStorage and useUser.



new package:
ui: