feat: allow user to change his password/email/name #20
Conversation
|
Your Render PR Server URL is https://rssmarkable-pr-20.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cf2pro94reb81vu9r1u0. |
src/components/common/Input.tsx
Outdated
| @@ -13,7 +13,7 @@ export const Input = forwardRef< | |||
| <label className="block text-sm font-medium text-gray-700"> | |||
| {children} | |||
| </label> | |||
| <div className="relative mt-1 rounded-md shadow-sm"> | |||
| <div className="relative rounded-md shadow-sm"> | |||
There was a problem hiding this comment.
You should check login and register page and other places where input is used, to check if something is illegible
| readonly initValue?: string; | ||
| } | ||
|
|
||
| export const UserSettingsRow = ({ |
There was a problem hiding this comment.
use memo here, please check CONTRIBUTING for component structure
| initValue, | ||
| }: UserSettingsRowProps) => { | ||
| const [value, setValue] = useState(initValue); | ||
| const [editMode, setEditMode] = useState(false); |
| </> | ||
| } | ||
| /> | ||
| ); |
There was a problem hiding this comment.
What's basically going on here? We should stick with our row structure and modify content from props, but all content and probably it should be determined by isEditing mode flag
| readonly children: ReactNode; | ||
| } | ||
|
|
||
| export const Action = ({ onClick, children }: ActionProps) => ( |
There was a problem hiding this comment.
Don't we have Button component? Is it different?
| </li> | ||
| ); | ||
|
|
||
| SettingsRow.Action = Action; |
| onChange={handleSelectChange} | ||
| > | ||
| {labels.map((label, i) => ( | ||
| <option key={i} value={i}> |
| <Tabs labels={labels} index={activeTab} onChange={setActiveTab} /> | ||
|
|
||
| {panels.map((panel, index) => ( | ||
| <Tabs.Panel key={index} index={index} value={activeTab}> |
There was a problem hiding this comment.
don't use index, what's value?
| Settings | ||
| </h2> | ||
|
|
||
| <Tabs labels={labels} index={activeTab} onChange={setActiveTab} /> |
There was a problem hiding this comment.
what's labels? they should be named tabs I guess
| key={i} | ||
| className={twMerge( | ||
| "cursor-pointer whitespace-nowrap border-b-2 py-4 px-1 text-sm font-medium", | ||
| i === index |
There was a problem hiding this comment.
Probably we should change active tab based on url param:
/settings -> general active
/settings/notifications -> notifications active etc.
|
|
||
| import type { HTMLAttributes } from "react"; | ||
|
|
||
| const variants = { |
| }; | ||
|
|
||
| type HeadingProps = Readonly<{ | ||
| level?: keyof typeof variants; |
There was a problem hiding this comment.
Why level is optional? And why we are passing 1 as default value?
src/views/dashboard/feeds/Feeds.tsx
Outdated
| <h2 className="text-lg font-medium leading-6 text-gray-900"> | ||
| All feeds | ||
| </h2> | ||
| <Heading level={3}>All feeds</Heading> |
There was a problem hiding this comment.
Why it's level 3 for example? I guess that our main title should be h1 indeed
src/views/dashboard/Home.tsx
Outdated
| @@ -141,9 +140,9 @@ export const HomeView = () => { | |||
| </div> | |||
|
|
|||
| <section className="mx-auto mt-10 max-w-6xl sm:px-6 lg:mt-12 lg:px-8"> | |||
| <h2 className="px-4 text-lg font-medium leading-6 text-gray-900 sm:px-0"> | |||
| <Heading level={3} className="px-4 sm:px-0"> | |||
There was a problem hiding this comment.
Our headings order is completely inappropriate here
Bartek532
force-pushed
the
main
branch
2 times, most recently
from
eb4a19e
to
bbf948e
Compare
| readonly onChange: (content: string) => void; | ||
| } | ||
|
|
||
| export const SettingsRow = ({ |
There was a problem hiding this comment.
put this component in /settings folder
| <div className="grow text-gray-800"> | ||
| {isEditMode ? ( | ||
| <Input | ||
| type={contentType ?? "text"} |
| label: "General", | ||
| pathname: "/dashboard/settings", | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
I will make /settings folder, move all components related to it there (without Settings prefix) and move something like this to consts file in /config
| import { Heading } from "../heading/Heading"; | ||
| import { SettingsRow } from "../settingsRow/SettingsRow"; | ||
|
|
||
| export const GeneralTab = () => { |
| "mt-1 flex items-start sm:col-span-2 sm:mt-0 sm:items-center"; | ||
| export const LEFT_SECTION_STYLES = "grow text-gray-800"; | ||
| export const RIGHT_SECTION_STYLES = | ||
| "ml-2.5 flex h-full flex-shrink-0 flex-col items-stretch gap-1 font-medium sm:flex-row sm:items-center"; |
There was a problem hiding this comment.
Why consts? Just put it into element directly
src/hooks/useYupForm.ts
Outdated
| onInvalid?: SubmitErrorHandler<TypeOf<T>>; | ||
| } | ||
|
|
||
| export const useYupForm = <T extends ZodType>( |
There was a problem hiding this comment.
Why we need this hook? And we are not using Yup in our app, btw
| readonly children: ReactNode; | ||
| } | ||
|
|
||
| export const Panel = ({ index, value, children }: PanelProps) => |
There was a problem hiding this comment.
For what we need this component?
| readonly label: string; | ||
| readonly schema: ZodObject<{ content: ZodString }>; | ||
| readonly contentType: HTMLInputTypeAttribute; | ||
| readonly content?: string; |
Bartek532
force-pushed
the
main
branch
5 times, most recently
from
40cc715
to
53c97d3
Compare
Bartek532
force-pushed
the
main
branch
3 times, most recently
from
011dc53
to
e38301d
Compare
Fixes #18
