MCKIN-7571 IE V2 hotspot detail pane

[murad-hubib]

• Added hotspot detail pane for image explorer V2

@kaizoku Please review and merge

Steps to check:

• Just click any hotspot and the details of the hotspot should appear in the pane on right side.

Ticket: https://edx-wiki.atlassian.net/browse/MCKIN-7571

CC: @bradenmacdonald @xitij2000

[openedx-webhooks]

Thanks for the pull request, @murad-hubib! I've created OSPR-2521 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

• supporting documentation
• edx-code email threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

We can't start reviewing your pull request until you've submitted a signed contributor agreement or indicated your institutional affiliation. If you like, you can add yourself to the AUTHORS file for this repo, though that isn't required. Please see the CONTRIBUTING file for more information.

[mduboseedx]

@murad-hubib Thank you for the contribution! @bradenmacdonald Does Murad have an agreement with OpenCraft about contributing code to Open edX? I understand that this is MCKIN ticket so it is a bit of a different scenario.

[bradenmacdonald]

@mduboseedx Well @murad-hubib works for Arbisoft so is covered under their agreement; OpenCraft will do the code review here.

[murad-hubib]

@kaizoku Thanks for the review. I have fixed that tutorial view height issue. The detail pane for the extended width issue is not occuring on my local machine. Can you please specify your browser and resolution?

[zamanafzal]

@kaizoku Please have a look into the changes @murad-hubib made. Also, I have added commit for https://edx-wiki.atlassian.net/browse/MCKIN-7572. As it can be done with it.

[oliviadunn]

@murad-hubib @kaizoku @zamanafzal - how is this looking? would like to wrap this up in this sprint. Thanks!

[kaizoku]

@murad-hubib Thanks for those fixes.

I'm using firefox, my overall screen resolution is 1920x1080. I'm not quite sure why that is loading below the image. I checked on another drag and drop block, and even when it's skinnier than the hotspot-detail pane, it sometimes shows up below the image:

[kaizoku]

@oliviadunn This is almost ready apart from the small styling issue I just described.

[murad-hubib]

@kaizoku I have tested it on my Linux machine with Firefox and my device resolution is also 1920x1080. I have tested it in firefox: 1- SDK 2- Studio 3- Mckinsey

and I am not able to produce this issue.

Are you using Macbook?

[kaizoku]

@murad-hubib I'm using Firefox 61.0.1 on debian.

The screenshot I provided is from the LMS, but I do also see the issue in studio and apros.

Also I notice you're using an xblock with a wider image. Do you have the test apros course? That has the image and problem setup which is failing for me.

I'll see if I can reproduce with the MIT dome photo which is the default image, but it could be that since the dimensions are larger, it won't have the same problem.

You could also try replacing the image with a smaller one and see if the problem can then be reproduced.

[murad-hubib]

@kaizoku Thanks and yes it was due to small image and it were creating margin in minus from left. I have removed the margins as we no longer need it. Please verify the change now and merge this PR. Thanks

[kaizoku]

That fixed the issue with the details loading below the image @murad-hubib. Thanks for updating that.

Sorry to tell you though, there are a couple other styling things I'm seeing still.

For some detail pains, the close button is outside of the xblock area and is hidden under the border:

When there is a video embedded, part of the video window is also outside of the viewable area:

It'd help the usability quite a bit if we could iron those out.

[murad-hubib]

@kaizoku I dont have any idea why "image-explorer-hostspot-reveal" class is getting these css properties (see attachment) it should have three properties i-e:

1- width should be auto but it is displaying width: 440px; 2- max-height should be 300px but the screenshot has height property instead 3- display property is alright

The close button is going outside the view just because of that 440px width. Can you please confirm that you are not tempering/adding the properties/markup in deubugger of browser?

The default video was carrying width 400px so I have changed it to 100%. it should now works fine.

[kaizoku]

Ah sorry @murad-hubib , I realize now that the sizes are because my problem was created before the sizes were removed from the default problem. So they still have the hardcoded sizes set in the OLX:

        <hotspot x='250' y='70' item-id="hotspotB">
            <feedback width='440' height='400'>
                <header>
                    <p>
                        Watch the Red Line subway go around the dome
                    </p>
                </header>

            </feedback>
        </hotspot>

In this case we can either leave things how they are, and if people set the width too wide, that's considered a misconfiguration. The other option, which I think would be a bit more safe is adding some CSS to prevent the width of the side panel from being greater than the parent:

.image-explorer-hotspot-reveal { max-width: 100%; }

If you think that's an alright solution, would you mind adding it @murad-hubib ? Otherwise we can probably merge this as is, it would just be nice to prevent people from running into this issue by accident.

[murad-hubib]

@kaizoku Thanks for finding the root cause. I have pushed the changes you asked lately. I have set both min and max width to 100%.

[kaizoku]

Great, thanks @murad-hubib . That fixed the issue for me. :+1:

• [✓] I tested this: (deployed in devstack and tested xblock with a variety of image sizes)
• [✓] I read through the code
• [✓] I checked for accessibility issues
• [n/a] Includes documentation

[murad-hubib]

Thanks @kaizoku Can you please also merge this PR?

[kaizoku]

@murad-hubib could you squash the commits so there aren't quite so many?

[murad-hubib]

@kaizoku Done

[kaizoku]

Thanks @murad-hubib. I've merged this PR!

[murad-hubib]

Thank @kaizoku Can you please make the release of this feature? and also we need to merge the PR on mcka-theme https://github.com/mckinseyacademy/mcka-theme/pull/16 to get this ticket moved to ready for testing.

[kaizoku]

@murad-hubib I merged the theme, and Kshitij is working on a release for this one.