-
Notifications
You must be signed in to change notification settings - Fork 296
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
Hide Rate dropdown menu when not applicable #1012
Hide Rate dropdown menu when not applicable #1012
Conversation
I want to ask that if this is the right way to address the issue? Overall, the code fixes the issue but I'm not sure if this is the approach that OED wants. |
Thanks to @nathang15 for another pull request. Overall, the idea worked well. I pushed a commit that includes the exclusion for graphics other than line from the rate menu since I think it makes them more consistent within the page and with other pages OED has. Please let me know if you think this change is good or something else should be done. I did notice one issue while testing the code and I don't think it is an issue you introduced but something that became apparent due to this change. When you hide the rate menu while still on the line graphic page and then allow it to reappear, the help for rate no longer pops up. If you leave this graphic page for another one and come back then it will work again. For example, select the unit kWh and then unselect it (click x) and the rate menu help will not work. I suspect it has to do with the fact that it was changed while still on the page but I have not tried to track it down carefully. If you want to look at this then great. If you want someone else to look at it then please let me know. |
Thank you for the updated changes. I think the new code makes it easier for other developers to understand. For the issue, may I ask that is the rate menu help refers to the tooltip? If so, I can take a look at the problem and fix it. Do I need to create a new issue for this or fix the newly occurred problem directly on this issue? Thank you. |
Yes, you cannot click the tooltip icon and get a pop up at times. I don't think we need a new issue but it would be nice to fix this before we merge this PR. You are welcome to look at this if you wish. Thanks. |
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 have tried locating where the issue occurs for a while, but I still cannot find where the exact problem happens. I have tried different approaches to identifies where exactly the problem happens but kind of unsuccessful.
Therefore, I resort to a "cheat" solution, which is to add a onClick event to rebuild the Tooltip. Essentially, after the Rate Menu is toggled between hidden and visible, the user would need to click twice for the tooltip to appear (which does not appear before the fix after the toggle event).
I believe that the underlying cause is because of some kind of bug in the React Tooltip package because the code for the tooltip component and its trigger does not look out of the ordinary.
Thank you.
The solution works reasonably well and only happens when you click the icon. I noted that you have to click the icon twice to get the help pop up and get the code to fire up when the issue occurs. I played with it a little and did not find a quick solution so I'm leaving this with a TODO when more global changes are made to tooltips. |
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.
Thanks to @nathang15 for another contribution to OED. The code works well and is ready to merge.
Description
Set the Rate dropdown menu in Line Graph to be hidden away when either kW appears on y axis or raw unit type is selected.
Fixes #899
Type of change
(Check the ones that apply by placing an "x" instead of the space in the [ ] so it becomes [x])
Checklist
(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)
Limitations