Integrate visual styling into the course outline #4549
Conversation
andy-armstrong
changed the title
|
@talbs Here's the new PR and branch with the cherry-picked commits... let me know if you see any problems with it. |
|
@andy-armstrong, I synced up the publishing-based color states to the Unit/Container views and also fixed a few things on your branch:
The latter fix brought up some display discrepancy with the static template and design comps. Here are screengrabs of both a Section and Subsection that has all of its content as staff_only with this branch's current rendering vs. the expected rendering: Section Subsection In both cases, the .status-release element's content needs to be updated to show the "Will never release - Contains Staff only content" and the .status-message is not needed on these levels (but is still needed for display with staff_only units). Let me know if you can help fix this and if you have any questions. |
|
Thanks @talbs. I can definitely address the issues you've brought up. |
There was a problem hiding this comment.
It would be good to do a PR of the LegacyPublishState rename into master (or else change it back?). I'm worried it will complicate rebases on master.
There was a problem hiding this comment.
I was talking this over with @benmcmorran and it would probably be better to actually remove compute_publish_state entirely since it can only be confusing our logic. I'll look at that and if it isn't possible then Ben will do the rename on master.
andy-armstrong
changed the title
|
@cahrens @benmcmorran @dan-f @explorerleslie @talbs @frrrances I've wrapped up all that I can do before I leave so hopefully things aren't too far from being merged. I've addressed all of Christina's comments and fixed the bug she found. I've updated my sandbox to the latest changes. There are a couple of known bugs:
http://studio.andy-armstrong.m.sandbox.edx.org/course/AndyA/PUB101/PUB101 Good luck everybody and I can't wait to get back from vacation to see it all live! |
|
FYI-- there seems to be an issue on the sandbox with the unit page-- it always gives a server error. There also appears to be at least one issue with the coloring algorithm on the overview page; if you create a new section (and subsection), the are blue, even though release date is in the future. I'll take a look at these bugs. Update: the sandbox just seems to be in a very bad state. I couldn't reproduce either of the issues above when I checked out the branch. |
|
@talbs Styling looks odd for empty course (the new section button). |
There was a problem hiding this comment.
Should also check that has_changes is False. Probably would be good to have a helper method to verify "published with no unpublished changes.
|
When you click to edit a section or subsection title, the Editor does not replace the non-editable text and is instead shown below it. |
|
@cahrens Ben knows about the problem with the inline editing. I should have mentioned that it is another styling breakage that happened when @benmcmorran fixed the code. He'll work it out with @talbs. |
|
@cahrens Sorry that I left the sandbox in a bad state, and thanks for fixing the issues that you've found so far. I've started another sandbox update that should be ready in the morning. |
There was a problem hiding this comment.
There do not appear to be any JavaScript unit tests that cover this logic (there is no course_outline_spec.js for the view version of course_outline.js, just the page version).
|
@cahrens I see that issue only on Chrome, not Safari or Firefox. |
|
@cahrens, I resolved some of the issues you noted previously around no outline content.
|
|
@cahrens, here is feedback to your pointed questions asked an hour ago...
I'm not able to reproduce this locally (either in Firefox or Chrome). The animations/transitions are being handled smoothly for me. I'm happy to take a look with @explorerleslie and you once you have an updated sandbox. Until then, I shall carry on.
The desired behavior is exactly what you are seeing - the primary action for any section/subsection is to collapse/expand its contents (hence the title portion of the UI firing that event) and the secondary action is to edit the title's value (that's why there is the pencil/edit icon to the right of the title when a user hovers). This was previously decided upon and signed off on by @frrrances, @explorerleslie, and @andy-armstrong to my knowledge. |
|
@cahrens, since you're taking point on this work. I noted a UI discrepancy for completely staff locked sections/subsections late last week I need your help on - https://github.com/edx/edx-platform/pull/4549#issuecomment-50090038. In short, the planned for display (both in the design comps and my static template) is a bit off. |
|
@cahrens, I've made sure the Unit/Container views are synced with the color-based state styling and differentiated the Unit Location UI element in that sidebar so it doesn't look like it communicates the same information as the publishing status UI element. |
|
@dan-f, thanks for the heads up RE: long section/subsection titles. I've made some accommodations for those cases now. |
|
Awesome |
|
@cahrens I know this isn't the right PR, but I can't find any open PRs on the unit page. Talked with @frrrances and we do need to add strikethrough back to the release date if "Hide from Students" is on. Frances is pretty sure if was working a couple weeks ago. |
There was a problem hiding this comment.
Thank you cleaning this up. So awesome.
|
@explorerleslie I have added back in the strike through. I will recreate Andy's sandbox. |
There was a problem hiding this comment.
If you are extending the placeholder, do you need to repeat the styles for margin, border, padding?
There was a problem hiding this comment.
NOPE! I'll remove that lil slice of redundancy. Thx!
|
@talbs, awesome work! I made a couple small comments that you can address or not. @cahrens, I don't think we want to inherit staff lock to subsections and sections since those will show up in the course nav on the release date. We should just show the release date and the default color bar, and only show the lock on the unit. Once that's addressed, 👍 |
|
@cahrens @talbs one more tweak to the course outline I just noticed (let me know if you want a separate story/bug reported). When you click to edit a section/subsection name now, the text for the current name should be highlighted, so that as soon as the user starts typing, the whole name is replaced with what the user is typing. Right now, you always have to manually select the text to replace, which is clunky. |
|
reported my above comment as https://openedx.atlassian.net/browse/STUD-2032 |
* syncing up unit publishing state UI with stateful names/styles * revising outline item status message display logic to show release status * fixing publishState value typo in outline UI template * refining and syncing incontext editor styling * maintaining visual alignment of collapsed/expanded sections in outline UI * simplifying page-level action styles on outline UI
* correcting visual display of outline UI without content * temporarily resolving page actions that should be hidden by default
Stop propagation of clicks in XBlockStringFieldEditor Fix collapsed subsections expanding when creating or deleting subsections Prevent clicking in display name textbox from toggling expand collapse Add test for published title when not live
* syncing up status-release hover states for outline units * adjusting in-context editor UI for longer outline titles * corrects outline incontext editor display regression * cleaning up unit/container sidebar styling * removing redundant margin on Outline UI subsections * selecting text when a user focuses into an incontext-editor input - STUD-2032
Fix failing bokchoy tests
Integrate visual styling into the course outline
STUD-1849