Updates from Settings Profile - Account Challenge #18
Conversation
src/reducers/profile.js
Outdated
| @@ -292,7 +292,6 @@ function onAddWebLinkDone(state, { payload, error }) { | |||
|
|
|||
| if (error) { | |||
| logger.error('Failed to add web link', payload); | |||
| fireErrorMessage('ERROR: Failed to add web link!'); | |||
There was a problem hiding this comment.
Why to remove UI error message in case of error? Without it user will have no idea, whether he should wait more, or operation failed, etc.
There was a problem hiding this comment.
because i added new error message keep behavior like existing code, https://monosnap.com/file/uNhRA5ZhBzOWXKwVCYXZDm7ZAiTVHp
you can confirm this add new weblink at https://www.topcoder.com/settings/profile/
fireErrorMessage will throw a global error message, that not looks good for user
There was a problem hiding this comment.
We can bend existing look and behavior to follow newer TC styles, and codebase. The problem is that doing so, you ask every user of profile actions / reducers to care himself about error reporting in the pages where these actions are used. And the original approach automatically ensures error reporting.
src/reducers/profile.js
Outdated
| @@ -342,7 +341,6 @@ function onLinkExternalAccountDone(state, { payload, error }) { | |||
|
|
|||
| if (error) { | |||
| logger.error('Failed to link external account', payload); | |||
| fireErrorMessage('ERROR: Failed to link external account!'); | |||
There was a problem hiding this comment.
src/reducers/ui/settings.js
Outdated
| @@ -39,9 +43,11 @@ function create(defaultState = initState) { | |||
| return handleActions({ | |||
| [a.profile.toggleTab]: (state, { payload }) => ({ ...state, currentProfileTab: payload }), | |||
| [a.tools.toggleTab]: (state, { payload }) => ({ ...state, currentToolsTab: payload }), | |||
| [a.account.toggleTab]: (state, { payload }) => ({ ...state, currentAccountTab: payload }), | |||
There was a problem hiding this comment.
Hmm... I also missed the point at which we started include UI actions / reducers into topcoder-react-lib, but it is not correct: unlike actions / reducers related to communication with Topcoder APIs, that can, and should be effectively reused between different Topcoder apps, UI-related actions / reducers belong only to the apps that need them, thus should not be placed into topcoder-react-lib.
There was a problem hiding this comment.
Hmm.. i think so. The first committer of this file is not me. so i just follow existing code to add new function for this part and keep consistent
There was a problem hiding this comment.
Ok, but as we have noticed it now, let's move it to the appropriate place?
There was a problem hiding this comment.
OK. I'll move ui actions / reducers to community-app, and put back removed error messages
|
@dedywahyudi I'd say, let's move ui actions / reducers to community-app, and put back removed error messages? |
|
@birdofpreyru Wait, I'll get back to you, checking with our developer. |
|
@birdofpreyru please check the updated commits. Let me know what you think. |
|
@dedywahyudi visually, looks good to me. Have you tested the update along with community-app? |
|
@birdofpreyru yes, tested and look good with community-app. |
@birdofpreyru
Here's the updates from Topcoder Settings Profile - Account Challenge
Attached the patch file. This PR already applied the patch.
develop.patch.zip
FYI: Not include the Email Verification bus Event yet.
cc @ThomasKranitsas @cwdcwd @urwithat