-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Pr05 Typescript Migration #21: client/modules/User
#3705
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: develop
Are you sure you want to change the base?
Pr05 Typescript Migration #21: client/modules/User
#3705
Conversation
…erificationTokenState
…alAuthServices enum
…useSyncFormTranslations to handle null formRef
…med export; update useSyncFormTranslations FormlikeType to take generic
…update to named export, update types
…opType & update to named export
client/modules/User
| if (!user.cookieConsent) { | ||
| user.cookieConsent = CookieConsentOptions.NONE; | ||
| } |
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.
fixes type-error flagged for in case user.cookieConsent is undefined
| * If using a native button, specifies on an onClick action | ||
| */ | ||
| onClick?: () => void; | ||
| onClick?: (evt: React.MouseEvent<HTMLButtonElement>) => void; |
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.
some callers want to pass a React.MouseEvent for evt.preventDefault
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.
I believe this isn't used anywhere anymore, it seems like a specific notification for a planned outage?
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.
Also don't think this is used anywhere anymore
| import { APIKeyForm } from '../components/APIKeyForm'; | ||
| import Nav from '../../IDE/components/Header/Nav'; | ||
| import ErrorModal from '../../IDE/components/ErrorModal'; | ||
| import { hideErrorModal } from '../../IDE/actions/ide'; |
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.
I can't tell if this was deleted since I wrote the PR originally and now it looks in the diff like I'm adding it back? I don't recall adding this in myself
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.
Ah ok I think I may have added this in to resolve a type-error that ErrorModal requires an closeModal prop, see here
Then I looked up where else usese ErrorModal and copied the implementation from IDEOverlays
Sorry was struggling to remember from October haha
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.
oh no worries!! thanks for the explanation haha
khanniie
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.
thank you so much for the great work!!
Context:
I was trying to record a demo of how I migrate files for the final presentation, and got a bit hyper-focused so I ended up migrating the majority of the
client/modules/UserfolderContains changes from #3702, #3703, #3704
I have also manually tested these forms to make sure nothing broke
Changes:
client/modules/User pages & components related to the User api
controlleras it's not migrated yetNotes:
Cherry-picked relevant commits from the giant rough-work PR here for easier review
Checks:
I have verified that this pull request:
npm run lint)npm run test)developbranch.Fixes #123