-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add option for email fields to set logged in users email as default #72
base: main
Are you sure you want to change the base?
Conversation
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.
This PR is really interesting, I like the feature it adds. Thanks!
I left a couple notes on improvements that can be done.
src/components/FormView.jsx
Outdated
); | ||
const dispatch = useDispatch(); | ||
useEffect(() => { | ||
if (userId?.length > 0 && curUserEmail === false) dispatch(getUser(userId)); |
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 think we have this info in redux already, if the user is logged in, right? No need to dispatch again if we do have it
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.
If the user is logged in we can retrieve the userId via (state) => jwtDecode(state.userSession.token).sub
, but this is not necessarily the same as the users email address (depends on the corresponding setting of the Plone instance).
For additional user info (like email) we have to dispatch getUser
.
Via 1c44b11 I make sure it is not dispatched, if another component already did it.
src/components/FormView.jsx
Outdated
) { | ||
const name = getFieldName(subblock.label, subblock.id); | ||
if (!formData.hasOwnProperty(name)) | ||
formData[name] = { value: curUserEmail }; |
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 recommend against doing it this way. It is considered a bad practice to mutate props and also to mutate state while rendering. All state changes should be done in a useEffect
callback and in this specific situation should be done using the onChangeFormData
function in order to properly handle state changes.
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 for the feedback! Resolved via ed8cf11
In order to use onChangeFormData
I had to compute fields_to_send_with_value
. This is done at an other location in the component as well. So I decided to create a reusable function getFieldsToSendWithValue
for it.
Co-authored-by: Piero Nicolli <pnicolli@users.noreply.github.com>
Co-authored-by: Piero Nicolli <pnicolli@users.noreply.github.com>
Adds checkbox to email fields
Use email of logged in user as default
.If selected and user is logged in, the field will get pre-filled with the users email address.