Yufei - make intangible time numbers having a mouseover explanation editable by the owner #1030

yufeiStella · 2023-07-22T00:51:08Z

Description

Related PRS (if any):

This frontend PR is related to the #454 backend PR.

Main changes explained:

Create mouseover text of total time component and add it into leaderboard component
Create mouseover text action, reducer, etc.

How to test:

check into current branch
do npm install and npm run start:local to run this PR locally
log in as an owner user and verify that the mouseover text appears when you move your mouse over both 'Total Time' and the numbers on the Leaderboard. Some numbers are blue (indicating intangible hours logged), and some numbers are black. Mouseover text will appear on both types of numbers.
Only the owner can see the edit button next to 'Total Time'. Click on it and enter the new text to update the mouseover text. Please note that it may take some time to respond, as shown in my screen recording.
Wait a few seconds for a green pop-up box to appear, showing the message "Mouseover Text updated!" Then you will see the newly updated mouseover text when you move your mouse over both 'Total Time' and the numbers. If you don't immediately see the changes, just wait a few more seconds, as it may take some time for the updates to respond.
Hard refresh the page to see the change is globally. The same thing is if you don't immediately see the changes, just wait a few more seconds, as it may take some time for the updates to respond.
login as any other role to verify the mouseoverText updates globally and they do not see the edit button.

Screenshots or videos of changes:

Log in as an owner:

Screen.Recording.2023-08-03.at.21.10.09.mov

Log in as any other user except owner:

Screen.Recording.2023-08-03.at.21.15.51.mov

Note:

Sometimes, you may need to wait for the page to respond and display the mouseover text. I encountered a similar situation as shown in my screenshot recording. When you move your mouse to another place where there should be mouseover text, you may experience the same situation. Just be patient and wait.

This reverts commit 8a26c65.

…s dynamic; in leaderboard: add id as new identifier for testing

xaanders

Hi @yufeiStella !
I have tested your PR. Functionality works as intended.
I could change mouseover text as an owner

As an admin or manager I could only read the explanation:

However I want to suggest rendering of the button based on permissions(and probably sending the update request as well):

scrnli_7_22_2023_10-41-35.AM.webm

igorrochadasilva · 2023-07-22T16:07:07Z

Hi @yufeiStella !
I hope you are well =D

I tested the PR, the admin I can see the mouseoverText, and can't see the edit button.

But when I tested logged in as Owner, I can't see the edit button, I'll try to see what happened, maybe something in my local environment

n0vay

Great Job!
Checked the file as an owner to edit the hover over text, was able to see it with admin and other roles account too.
One small suggestion, you should include the steps to change backend in the testing instructions, I was a bit confused when it was not working.

AuroraLovesWhales

Hi @yufeiStella. I have tested your PR and it is working as expected. Great job!

Log in as Owner, edit the mouseover text for total time, and refresh the page:

Screen.Recording.2023-07-22.at.6.42.18.PM.mov

Log in as any users other than Owner:

AaronPersaud

Hi, I've tested it and it works as expected!

1030.mp4

(I realized after the video didn't record cursor hovering and displaying the text)

I tested the following:

non owners cannot see icon for editing
both textareas show up for all users
owner can edit and save text
text is updated on refresh and when switching to another page and back

I have added some code suggestions that would make it cleaner and a bit more concise.

AaronPersaud · 2023-07-23T01:18:32Z

src/components/LeaderBoard/Leaderboard.jsx

+ {isOwner && (
+ <MouseoverTextTotalTime onUpdate={handleMouseoverTextUpdate} />
+ )}
+ {!isOwner && (


Like @xaanders mentioned, it's not necessary to render the hidden button if the user isn't owner. You can probably remove 224-228.

Thank you! Updated my code. The thing is that I still need to fetch updated mouseover text, but now I have changed to the following code:

MouseoverTextTotalTimeUpdater is just to fetch updated data, not a button with update function.

Thank you, looks good.

AaronPersaud · 2023-07-23T01:24:53Z

src/components/mouseoverText/mouseoverTextTotalTime.jsx

+ if (text) {
+ updateMouseoverText(mouseoverTextId, mouseoverText);
+ setModalOpen(false);
+ setText(newText);
+ onUpdate(newText);
+ } else {
+ //console.log('createMouseoverText is ' + mouseoverText.newMouseoverText);
+ createMouseoverText(mouseoverText);
+ setModalOpen(false);
+ setText(newText);
+ onUpdate(newText);
+ }


line 49-51 is the exact same as 55-57, so it's a bit redundant to use the same logic twice. Since those steps happen regardless of the scenario, you can instead put that outside of the if/else blocks.

Suggested change

if (text) {

updateMouseoverText(mouseoverTextId, mouseoverText);

setModalOpen(false);

setText(newText);

onUpdate(newText);

} else {

//console.log('createMouseoverText is ' + mouseoverText.newMouseoverText);

createMouseoverText(mouseoverText);

setModalOpen(false);

setText(newText);

onUpdate(newText);

}

if (text) {

updateMouseoverText(mouseoverTextId, mouseoverText);

} else {

//console.log('createMouseoverText is ' + mouseoverText.newMouseoverText);

createMouseoverText(mouseoverText);

}

setModalOpen(false);

setText(newText);

onUpdate(newText);

All set, thank you for suggestion!

Hi @AaronPersaud I've just updated this PR. Could you please re-review my PR and approve it if you agree with the changes? Thank you!

shiva2096

Hi @yufeiStella

Great work! Functionality is working as expected.

Screen.Recording.2023-07-22.at.11.15.50.PM.mov

winghojackyli

I tested your PR and it worked as expected.
I logged in as owner and I can change the mousehover text from "text" to "text by owner".

Then, I logged in as admin and I do not have the edit mousehover text button, but I can see the edited mousehover text.

I logged in as volunteer and it worked as expected as well.

AnishPandita · 2023-07-25T00:37:33Z

Hi @yufeiStella it seems to work correctly. Edited using Owner account and verified by admin account as well.

DavidC0126

Hi @yufeiStella, I have tested your PR and everything works as intended!
I am able to see and edit the mouseover text as as owner, and I can only see the edited text but not able to edit the test as any other role.

Owner perspective - edit icon and ability to adit the text

Admin role perspective => can see but cannot edit it

beblicarl · 2023-07-25T08:52:51Z

I tried several times but I was unsuccessful in getting this to work as intended. Also, there were errors in the backend console which could be related to this issue.

You would notice from the video that even after editing the field with different texts the text that shows onMouseOver remains "some text:text"

screencast-localhost_3000-2023.07.24-17_04_18.webm

eduardocodes

Disconsidering the slow local host, this PR works as intended. Great job!

mouseover_test.mp4

sxiong5

Hey, the PR worked as intended.

Screen.Recording.2023-07-25.at.16.45.20.mov

sxiong5 · 2023-07-25T20:54:24Z

src/reducers/mouseoverTextReducer.js

+ switch(action.type) {
+ case types.GET_MOUSEOVER_TEXT:
+ const data = action.payload;
+ return data;
+
+ case types.CREATE_MOUSEOVER_TEXT:
+ return state;
+
+ case types.UPDATE_MOUSEOVER_TEXT:
+ return state;
+
+ default: 
+ return state;


The switch seems unnecessary here. Consider the following code, which makes the code more readable now unless you have the plan to add more operations to other types.

if (action.type === types.GET_MOUSEOVER_TEXT) { return action.payload; } return state;

Shreyj14 · 2023-07-26T22:14:41Z

Hi @yufeiStella

Reviewed this PR with backend 454, and it works as intended.

yufeiStella · 2023-07-27T04:52:23Z

I tried several times but I was unsuccessful in getting this to work as intended. Also, there were errors in the backend console which could be related to this issue.

You would notice from the video that even after editing the field with different texts the text that shows onMouseOver remains "some text:text"

screencast-localhost_3000-2023.07.24-17_04_18.webm

Hi @beblicarl, I'm not sure if the error is related to my functionality, but it does have a long response time or even have to reload it again when running the app. Besides, I noticed that you don't have any warnings after running 'npm start' and so you can receive the feedback in the backend console. However, every time I run 'npm start', I receive the warning (Use node --trace-deprecation ... to show where the warning was created" after running 'npm run build' and 'npm start' in the backend. The application can still run, but then I can't see any feedback in the backend console when running the application on localhost. Have you ever had the same warning (because I know someone else also have the same problem), and how did you resolve it? Or have you never encountered such a problem before?

Updated:
Hi@beblicarl I believe you are also aware of the urgent problem in Slack, and the errors are not related to my PR. The thing is that my PR should be reviewed in the dashboard. I have also updated my PR. Could you please re-review my PR again and approve it if you agree with the changes? I noticed that the application is working much more smoothly today!

iTvX · 2023-07-27T18:54:49Z

I have checked your PR. The owner can change the text, and it looks good. But when I switch accounts (not the owner's account), I cannot see the text. It seems like it's not global.

output.mp4

yufeiStella · 2023-08-02T05:21:18Z

Hi @yufeiStella ! I have tested your PR. Functionality works as intended. I could change mouseover text as an owner

As an admin or manager I could only read the explanation:

However I want to suggest rendering of the button based on permissions(and probably sending the update request as well):

scrnli_7_22_2023_10-41-35.AM.webm

Hi @xaanders, now I have rendered the button based on permissions(also the updating request because it is all about the edit button)! Could you please re-review it again and approve it if you agree with it? Thank you!

Screen.Recording.2023-08-02.at.00.20.08.mov

…ing_mouseover_explanation merge with development

beblicarl · 2023-08-02T18:33:26Z

The functionality works as intended and the code is great!

screencast-bpconcjcammlapcogcnnelfmaeghhagj-2023.08.02-18_28_12.webm

AaronPersaud

Hi Yufei, I have tested it and it looks good!

Thank you for addressing my code suggestions, they look great.

Nice work!

xaanders

Hi @yufeiStella! I've tested your changes. It works perfect!
I have just one suggestion to the code:

xaanders · 2023-08-04T15:06:41Z

src/components/LeaderBoard/Leaderboard.jsx

+ getMouseoverText();
+ }, [totalTimeMouseoverText]);
+
+ useEffect(() => {


I would suggest to make just one useEffect because they have the same dependency anyway

Thank you! Fixed.

yufeiStella added 4 commits July 7, 2023 23:24

first part

8d6d927

add mouseoverText on Total Time

39e08fd

Resolved merge conflicts with development

a5bae7c

save files

8a26c65

yufeiStella mentioned this pull request Jul 22, 2023

Yufei - Add a mouseover text edit button near 'Total Time' OneCommunityGlobal/HGNRest#454

Merged

yufeiStella added the do not review label Jul 22, 2023

yufeiStella added 5 commits July 21, 2023 20:08

Revert "save files"

b0a1332

This reverts commit 8a26c65.

keep leaderboard.css same as development to solve merge problem

59114ea

manually add changes to current Leaderboard.jsx to solve merge problem

8a981b1

remove extra commented code

071157e

in test: replace title with id to match element since now the title i…

b023884

…s dynamic; in leaderboard: add id as new identifier for testing

yufeiStella removed the do not review label Jul 22, 2023

xaanders suggested changes Jul 22, 2023

View reviewed changes

AaronPersaud self-requested a review July 22, 2023 21:54

n0vay reviewed Jul 22, 2023

View reviewed changes

AuroraLovesWhales previously approved these changes Jul 23, 2023

View reviewed changes

AaronPersaud suggested changes Jul 23, 2023

View reviewed changes

shiva2096 previously approved these changes Jul 23, 2023

View reviewed changes

beblicarl self-requested a review July 24, 2023 15:26

winghojackyli previously approved these changes Jul 24, 2023

View reviewed changes

DavidC0126 previously approved these changes Jul 25, 2023

View reviewed changes

eduardocodes previously approved these changes Jul 25, 2023

View reviewed changes

sxiong5 previously approved these changes Jul 25, 2023

View reviewed changes

Shreyj14 previously approved these changes Jul 26, 2023

View reviewed changes

yufeiStella dismissed stale reviews from eduardocodes, DavidC0126, winghojackyli, shiva2096, and AuroraLovesWhales via 8b25df5 August 2, 2023 04:56

yufeiStella requested review from AaronPersaud and xaanders August 2, 2023 05:06

Merge branch 'development' into yufei_make_intangible_time_number_hav…

df8709f

…ing_mouseover_explanation merge with development

beblicarl previously approved these changes Aug 2, 2023

View reviewed changes

AaronPersaud previously approved these changes Aug 3, 2023

View reviewed changes

add toast success message

ed6f115

yufeiStella dismissed stale reviews from AaronPersaud and beblicarl via ed6f115 August 3, 2023 19:29

yufeiStella added 3 commits August 3, 2023 14:50

update file name

e98fec1

change MouseoverTextTotalTimeEditButton.jsx file

ee21a8e

change useEffect part code

13180c9

yufeiStella added the do not review label Aug 3, 2023

add getMouseoverText into leaderboard.container.jsx

5ab4dc4

yufeiStella removed the do not review label Aug 4, 2023

xaanders suggested changes Aug 4, 2023

View reviewed changes

combine useEffects with the same dep

3d41c6b

yufeiStella requested a review from xaanders August 4, 2023 17:49

xaanders approved these changes Aug 4, 2023

View reviewed changes

one-community added the Moved to Final Review label Aug 4, 2023

one-community merged commit 56cc802 into development Aug 4, 2023
1 check passed

This was referenced Aug 4, 2023

Revert "Yufei - make intangible time numbers having a mouseover explanation editable by the owner" #1086

Merged

Frontend Release to Main [2.49] #1092

Merged

one-community mentioned this pull request Jul 17, 2024

Frontend Release to Main [3.66] #2458

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Yufei - make intangible time numbers having a mouseover explanation editable by the owner #1030

Yufei - make intangible time numbers having a mouseover explanation editable by the owner #1030

yufeiStella commented Jul 22, 2023 •

edited

Loading

xaanders left a comment

igorrochadasilva commented Jul 22, 2023

n0vay left a comment

AuroraLovesWhales left a comment

AaronPersaud left a comment

AaronPersaud Jul 23, 2023

yufeiStella Aug 2, 2023 •

edited

Loading

AaronPersaud Aug 3, 2023

AaronPersaud Jul 23, 2023

yufeiStella Aug 2, 2023

yufeiStella Aug 2, 2023 •

edited

Loading

shiva2096 left a comment

winghojackyli left a comment

AnishPandita commented Jul 25, 2023

DavidC0126 left a comment

beblicarl commented Jul 25, 2023

eduardocodes left a comment

sxiong5 left a comment

sxiong5 Jul 25, 2023 •

edited

Loading

Shreyj14 commented Jul 26, 2023

yufeiStella commented Jul 27, 2023 •

edited

Loading

iTvX commented Jul 27, 2023

yufeiStella commented Aug 2, 2023

beblicarl commented Aug 2, 2023

AaronPersaud left a comment

xaanders left a comment

xaanders Aug 4, 2023

yufeiStella Aug 4, 2023

Yufei - make intangible time numbers having a mouseover explanation editable by the owner #1030

Yufei - make intangible time numbers having a mouseover explanation editable by the owner #1030

Conversation

yufeiStella commented Jul 22, 2023 • edited Loading

Description

Related PRS (if any):

Main changes explained:

How to test:

Screenshots or videos of changes:

Note:

xaanders left a comment

Choose a reason for hiding this comment

igorrochadasilva commented Jul 22, 2023

n0vay left a comment

Choose a reason for hiding this comment

AuroraLovesWhales left a comment

Choose a reason for hiding this comment

AaronPersaud left a comment

Choose a reason for hiding this comment

AaronPersaud Jul 23, 2023

Choose a reason for hiding this comment

yufeiStella Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

AaronPersaud Aug 3, 2023

Choose a reason for hiding this comment

AaronPersaud Jul 23, 2023

Choose a reason for hiding this comment

yufeiStella Aug 2, 2023

Choose a reason for hiding this comment

yufeiStella Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

shiva2096 left a comment

Choose a reason for hiding this comment

winghojackyli left a comment

Choose a reason for hiding this comment

AnishPandita commented Jul 25, 2023

DavidC0126 left a comment

Choose a reason for hiding this comment

beblicarl commented Jul 25, 2023

eduardocodes left a comment

Choose a reason for hiding this comment

sxiong5 left a comment

Choose a reason for hiding this comment

sxiong5 Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

Shreyj14 commented Jul 26, 2023

yufeiStella commented Jul 27, 2023 • edited Loading

iTvX commented Jul 27, 2023

yufeiStella commented Aug 2, 2023

beblicarl commented Aug 2, 2023

AaronPersaud left a comment

Choose a reason for hiding this comment

xaanders left a comment

Choose a reason for hiding this comment

xaanders Aug 4, 2023

Choose a reason for hiding this comment

yufeiStella Aug 4, 2023

Choose a reason for hiding this comment

yufeiStella commented Jul 22, 2023 •

edited

Loading

yufeiStella Aug 2, 2023 •

edited

Loading

yufeiStella Aug 2, 2023 •

edited

Loading

sxiong5 Jul 25, 2023 •

edited

Loading

yufeiStella commented Jul 27, 2023 •

edited

Loading