search

openedx / xblock-image-explorer

GNU Affero General Public License v3.0
9 stars 39 forks source link

hotspot details will come in sidebar #44

Closed zamanafzal closed 6 years ago

zamanafzal commented 6 years ago

@xitij2000 Can you please review this?

viadanna commented 6 years ago

Hello @zamanafzal I'm Paulo. I'm taking the review in place of @xitij2000

It seems to work fine in Studio but renders out of view in LMS, is this the expected behavior? image-explorer

zamanafzal commented 6 years ago

@viadanna yes this is expected behaviour in the new design. Please review it at https://github.com/zamanafzal/xblock-image-explorer/pull/1 This is just the functionality part ticket. I am creating a new branch named Image-explorer-v2 for this. I will open this new design and functionality related work against this branch for better understanding and visibility and at the end will merge that in master.

viadanna commented 6 years ago

Ok then, @zamanafzal This is good to go @xitij2000

xitij2000 commented 6 years ago

@zamanafzal I have created a branch called image-explorer-v2 in this repository for that purpose, so you don't need to merge in your fork. Please change the target branch of this PR to this new branch, and I will merge it there. In the end, as you said we will have a look at the final product and merge to master.

zamanafzal commented 6 years ago

@xitij2000 I have created a new PR with image-explorer-v2 branch https://github.com/edx-solutions/xblock-image-explorer/pull/45 Please merge that

xitij2000 commented 6 years ago

@zamanafzal Just FYI, you can change the title and target branch of a PR by click on "Edit" on the top, so you didn't need a new PR. I have merged #45 for now so closing this one.