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

Problem Bank "Add Components" modal is way too long in unit page iframe. #1502

Open
bradenmacdonald opened this issue Nov 14, 2024 · 10 comments
Assignees
Labels
bug Report of or fix for something that isn't working as intended

Comments

@bradenmacdonald
Copy link
Contributor

If you have a unit with many components and a problem bank on the NEW MFE unit page (with an iframe), clicking "Add Components" will open a modal that's way too tall.

This only affects the new unit page, not the legacy page.

Note that we are planning to overhaul this workflow in #1096 but that work is delayed until we get more use feedback.

Screenshot 2024-11-14 at 11 21 46 AM

Screenshot 2024-11-14 at 11 21 19 AM

@bradenmacdonald bradenmacdonald added the bug Report of or fix for something that isn't working as intended label Nov 14, 2024
@bradenmacdonald bradenmacdonald moved this to In grooming in Libraries Overhaul Nov 14, 2024
@navinkarkera
Copy link
Contributor

@bradenmacdonald Should we intercept the Add components button click and open the modal in MFE similar to #1464. I am almost done with #1464 and this should be similar.

@bradenmacdonald
Copy link
Contributor Author

@navinkarkera If it's an easy change, I think that would be a nice fix.

@navinkarkera
Copy link
Contributor

@bradenmacdonald Yes, we can show the modal on MFE side and still handle the adding blocks part in legacy by sending message to the iframe.

@navinkarkera
Copy link
Contributor

@jmakowski1123 @lizc577 @sdaitzman @marcotuts This is ready for AC testing on the sandbox

@navinkarkera navinkarkera moved this from In grooming to Ready for AC testing in Libraries Overhaul Nov 23, 2024
@sdaitzman
Copy link

Looks good to me!

image

@navinkarkera
Copy link
Contributor

@sdaitzman Same as #1464, this ticket should be tested from the new course unit. You can test it using this link.

The sandbox is set to take you legacy course unit page by default, but we flip this temporarily if required.

@sdaitzman
Copy link

Thanks @navinkarkera! It looks to me like there's still a pretty large gap between the last component in the unit and the Add a new component section, but no unusually tall modal when adding library components to a problem bank. I added a bunch of short components and one really long one to that unit for testing.

One other minor visual nit: I noticed a slight difference in color between the unit contents background #f5f5f5 and the surrounding page background #f8f7f6 in the screenshot attached.

image

@navinkarkera
Copy link
Contributor

it looks to me like there's still a pretty large gap between the last component in the unit and the Add a new component section, but no unusually tall modal when adding library components to a problem bank. I added a bunch of short components and one really long one to that unit for testing.

@sdaitzman This task is about handling the modal that is displayed on clicking Problem Bank button, so the gap between in components and the Add new component section is not related. (it is part of course unit page).

One other minor visual nit: I noticed a slight difference in color between the unit contents background #f5f5f5 and the surrounding page background #f8f7f6 in the screenshot attached.

Same for this one, it is related to course unit page. I think @PKulkoRaccoonGang can help here.

@PKulkoRaccoonGang
Copy link
Contributor

Hello everyone! At the moment, we have an intermediate version of the modal window functionality on the course unit page. Its full implementation will be added in future PRs.

It looks to me like there's still a pretty large gap between the last component in the unit and the Add a new component section

This indent will disappear in future PRs, it will be calculated dynamically depending on whether the xblock is the last one. The first steps to improve this were made in this PR #1431.

One other minor visual nit: I noticed a slight difference in color between the unit contents background #f5f5f5 and the surrounding page background #f8f7f6 in the screenshot attached.

Good catch! I will fix this 💯

@ChrisChV
Copy link
Contributor

@jmakowski1123 @bradenmacdonald is it okay to move this to Done?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report of or fix for something that isn't working as intended
Projects
Status: Ready for AC testing
Development

No branches or pull requests

5 participants