-
Notifications
You must be signed in to change notification settings - Fork 80
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
Within libraries, open editors in a modal, not a new URL #1321
Comments
Addresses an item in: openedx/edx-platform#35256 , openedx/edx-platform#35257 , and openedx/edx-platform#34692 |
@kdmccormick I'll see what I can do when I work on this, but because we're still using the legacy Unit page, it's likely that this change is only going to be an improvement for people using these editors from a content library, and not yet from a course. |
I think that's reasonable. Plus, it gives people a reason to look forward to the new course unit editor. |
@jmakowski1123 @sdaitzman @lizc577 This is ready for AC testing on the sandbox. |
Looking great. This is a super tiny nit, and in no way a blocker. When I click on any component, there's a super brief spinner that pops up before the modal. It's performing so quickly on my instance that it's so quick, you can barely process what you're seeing, but it's a little jarring. We definitely need a spinner or some indicator of loading if there's a delay, but when it opens so quickly, it's a bit jarring. Is there any solution for that? Some microsecond threshold where no spinner displays? Again, tiny nit. |
@jmakowski1123 I'm sure there's something we can do but it's not a trivial fix. Let's take a look after MVP ? We can always fix it for Sumac as a bugfix. |
Totally fine |
Looks good to me, the modal is great. Two small UX nits:
(There are some potential further UX improvements to this, like caching edits across reloads, but I think those are well post-MVP scope) |
@sdaitzman Those are excellent points. Neither of those issues are new - they've been around since the MFE editors were introduced (in courses). I can definitely fix both of them, but I'm not sure how to prioritize them relative to our other work on the MVP. |
@bradenmacdonald I'll defer to @jmakowski1123 on the level of priority/whether fixing these MFE issues is in-scope as part of this work. The unnecessary confirmation would appear more frequently for users regularly opening components for editing, but is basically just a small annoyance. Most users can expand the component preview as a workaround for MVP. The browser navigation issue would probably not occur very often, but would have a larger impact each time, since users could lose their recent edits. |
@bradenmacdonald I'm seeing a strange thing now when I paste Problem components into a Content Library: no matter what their problem type is, when I Edit the new pasted library problem, it always shows the OLX in the Advanced problem editor instead of using the nice problem-type-specific editor. I've reproduced this on tagging-preview too (see e.g. "Multiple Choice" problem in lib:OpenCraft:TL). Is this already a known issue? I can address next sprint as part of FAL-3873. |
@pomegranited No, I wasn't aware of that issue. It sounds similar to #1326 though. If you can fix it as part of FAL-3873, that would be great! |
I noted this was happening here (#1091 (comment)) - it seems to have been connected to the max attempts setting, but I couldn't say for sure that was the pattern. |
I wouldn't prioritize either of these in front of Epic 6 for the MVP. I think it could be argued that both of these could be classified as bugs. Just created two bug tickets and we can prioritize them against all the other library bugs during the 6 week testing period. Agree the navigation issue carries more weight. |
@jmakowski1123 With those two changes now made into separate tickets, can we mark this as done? |
I was waiting until the legacy editor reversion issue was resolved, which I realize is being tracked separately, but is tied to tests on this ticket. Is that bug fixed? |
#1377 Tracking this bug separately, so closing this issue. |
As an author, when I close out of an editor modal I expect to be right back where I just was, not taken to the library homepage.
As an author, I want to see at least a little bit of context when I'm editing a component. Example:
(Note how the course, section, and subsection are visible)
The text was updated successfully, but these errors were encountered: