Skip to content
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

Full interaction response tooltip misplaced for graph input interaction (and the image doesn't show up) #13699

Open
U8NWXD opened this issue Aug 18, 2021 · 6 comments · May be fixed by #20147
Open
Assignees
Labels
bug Label to indicate an issue is a regression Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Low Solution is known and broken into good-first-issue-sized chunks.

Comments

@U8NWXD
Copy link
Member

U8NWXD commented Aug 18, 2021

Describe the bug
The full interaction response tooltip is not alligned with the short interaction response that the user clicks on to show the tooltip

To Reproduce
Steps to reproduce the behavior:

  1. (optional) Create an interaction with the graph input interaction
  2. Open an interaction with the graph input interaction
  3. Submit an incorrect answer
  4. View your previous answers
  5. Click on a previous answer

Observed behavior

The tooltip is not aligned with the previous answer that the user clicked on. (EDIT: note that the image also doesn't show up, see #13699 (comment) too):

Expected behavior

The tooltip should be aligned.

Desktop (please complete the following information; delete this section if the issue does not arise on desktop):

  • OS: macOS
  • Browser: Chrome
  • Browser-version: 92
@U8NWXD U8NWXD added this to Awaiting Triage in Learner and Creator Experience via automation Aug 18, 2021
@kevintab95 kevintab95 added the bug Label to indicate an issue is a regression label Sep 1, 2022
@nithinrdy nithinrdy removed this from Awaiting Triage in Learner and Creator Experience Sep 4, 2022
@Lawful2002 Lawful2002 added Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Low Solution is known and broken into good-first-issue-sized chunks. labels Dec 15, 2022
@SubhashThenua
Copy link
Contributor

Hi @AdityaDubey0 Are you still workiing on this issue ?
Can you please give update ?
thanks!

@ana-mc-almeida
Copy link

Hello @seanlip @SubhashThenua !
I have been working on this issue, and would like to propose a solution. In the input-response-pair.component, I changed the code where the pop-up is being called. Below is proof of this correction.

Screenshot_20240327_155007

In addition to this, the graph SVG is not appearing at all (print from production environment). I would like to suggest that a new issue is openned, in order to solve this problem, since this one only refers to the misplaced tooltip.

Screenshot_20240327_151646

@seanlip
Copy link
Member

seanlip commented Mar 27, 2024

Thanks @ana-mc-almeida! I think we need to know more about what code you changed and what was causing the original bug.

Thanks also for noticing that the tooltip image is not showing up. I think this needs to be fixed as part of this, actually, since there's not much point making the tooltip fix without it. Could you please look into it too? I will update the issue title.

Thanks!

@seanlip seanlip changed the title Full interaction response tooltip misplaced for graph input interaction Full interaction response tooltip misplaced for graph input interaction (and the image doesn't show up) Mar 27, 2024
@ana-mc-almeida
Copy link

Hello @seanlip, I managed to find out why the image wasn't showing up (and fix it).

Here's an image with the final result of my changes:
Screenshot_20240402_115954

Regarding the code changes I had to make, they were as follows:

  • Positioning the tooltip correctly:

    • Moved the popup call inside the oppia-interaction-display component (previously it was in the parent div):
      Screenshot_20240402_120548
    • To fix the popup at the top of the message, it was necessary to remove the overflow property and replace it with margin-top:
      Screenshot_20240402_120841
  • Making the image appear:

    • Instead of calling a rich text component, I believe the oppia-interaction-display component should be called, and for that, I needed to call the detectChanges() method in the ngAfterViewInit lifecycle hook:
      Screenshot_20240402_121305
      Screenshot_20240402_121637
    • However, the image was poorly positioned inside the popup, so I made the following style changes in the graph-input-response.component:
      Screenshot_20240402_121754
    • After these changes, I also suggest increasing the value of GRAPH_INPUT_LEFT_MARGIN (from 120 to 200, for example) to reduce the white space, as demonstrated in the following image:
      Screenshot_20240402_122146

These were all the changes I made to the code to fix this issue. Let me know if I can help with anything else!

@seanlip
Copy link
Member

seanlip commented Apr 3, 2024

Great, thanks @ana-mc-almeida, I'll assign you! When can you create a PR?

Also, one note -- when you provide the "proof of changes" video, please verify that the answer display looks fine for all other interaction types as well (drag-and-drop, number, interactive map, etc.) -- I'm not sure which show a tooltip and which don't, but we should make sure that your changes don't break anything since they apply to generalized components. You can load the all_interactions exploration from /admin > Activities in your dev server (I think the exp ID is '16') and play through it, that should cover most of them.

Thanks!

@ana-mc-almeida
Copy link

Hi @seanlip, thanks for your help, I will verify the answer display in all the others interactions types.

I intend to create the PR as soon as possible. I'm currently having difficulties creating a test that verifies that the graph image is being displayed in the popup, but once it's done I'll create the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Label to indicate an issue is a regression Impact: Medium Will improve quality-of-life for at least 30% of users. Work: Low Solution is known and broken into good-first-issue-sized chunks.
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

8 participants