WIP - Studio: Group Access Level UI by talbs · Pull Request #6095 · openedx/openedx-platform

talbs · 2014-12-01T15:04:36Z

UX support for statically standing up and styling new UI within containers/components to support group access levels.

Background

JIRA Story/Spec - https://openedx.atlassian.net/browse/TNL-652
UI Design - http://invis.io/K21UDOV7H
Sandbox Unit Page Example - http://studio.talbs.m.sandbox.edx.org/container/i4x://edX/DemoX/vertical/vertical_0270f6de40fc

Screenshot - Combined Modal UI

Screenshot - Container/Unit View Tweaks

explorerleslie · 2014-12-02T18:34:44Z

@andy-armstrong I did find this PR for the the UX side of TNL-652 (assigning components to content groups).

talbs · 2014-12-03T14:05:04Z

@andy-armstrong and @explorerleslie, yes, sorry. This PR is a WIP for the access level settings. My commit messages should give you a clue what's been accounted for thus far. The one remaining item is the final modal UI.

talbs · 2014-12-10T00:22:22Z

Status Update: This work should be ready for engineering to pick up and for a UI/FED-focused review.

@explorerleslie, @catong, and @andy-armstrong, this branch now contains all of the static UI we went over today in our hand off/review meeting. I've also addressed the good feedback you had and decision we made as part of that meeting (in a separate follow-up commit - edx@75d7531).

I'll be spinning up a sandbox so its easier for you folks to see the very very very static UI rendering in the browser as well as for the UI/Design portion of my team's review.

A few notes:

This UI is very static and the modal I've roughed out contains all of the elements needed for all of the states combined (it is not a 1:1 representation of the UI planned).
@andy-armstrong, I've made  comments in places where I needed to explain the current state or a specific piece of logic I need your or an developer's help with. Search is your friend :)
The modal set here relies on the existing Studio modal set up and since its static does not leverage the JS-based window centering (i.e. it may not be truly centered in your browser).

A few asks:

@andy-armstrong, if someone else on your team picks this up, can you provide them with any light context we've already gone over and your insight into what that means from an engineering perspective?
@clrux, can you review my HTML, Sass/CSS, and FED work here?
@frrrances, can you do the same as well as review my UI/Visual design work?
@catong, if you have copy recommendations, feel free to note them in this code, in a separate commit, or on the wireframes (http://invis.io/K21UDOV7H) as Invision App notes.

I'll address any revisions here or in a developer's working branch depending on timing. Thanks for the support!

talbs · 2014-12-10T00:27:21Z

NOTE: these are redundant rules for checkboxes/radio buttons - I could move them into a Sass extend or define them both with a 2 selector rule if this offends anyone's religion :)

If the same rules will apply, what about something like:

.input-tickable { // Generic rules for checkboxes and radios }

And then add specificity like:

.input-tickable { .checkbox { } .radio { } }

As needed?

Good point! I can abstract, but my preference would be to make it a Sass placeholder (something like %input-checkable {}) that we can extend (@extend %input-checkable;) in these two cases vs. adding another class to the HTML. Sound okay?

I wonder if 'checkable' is the best term. You 'tick' both checkboxes and radios, but you only really check a checkbox. I wonder if 'tickable' would be best?

But yes, I like your approach.

talbs · 2014-12-10T00:33:23Z

NOTE: @andy-armstrong, please remove this when its not needed for static UI rendering.

downzer0 · 2014-12-10T12:49:43Z

@talbs I've made a pass through the code and added a few thoughts. I'm not very familiar with "xblocks" so I don't know how valid my thoughts are. Let me know if you want to discuss.

downzer0 · 2014-12-10T15:14:57Z

Does file need to be in quotes?

Sure does. Good catch on some existing code. On it!

talbs · 2014-12-10T17:46:31Z

Update: I finally got a sandbox up. You can view the static unit FED/UI in all of its modal glory here - http://studio.talbs.m.sandbox.edx.org/container/i4x://edX/DemoX/vertical/vertical_0270f6de40fc

talbs · 2014-12-10T21:35:32Z

@clrux and @explorerleslie, thanks for the UI and FED feedback - I've pushed those small tweaks to this PR and kicked off another sandbox build. You should be able to see the small visual changes in a bit.

catong · 2014-12-10T22:19:42Z

Typo: visible

talbs · 2014-12-11T18:48:57Z

@andy-armstrong, I've revised the markup/styling of the modal based on our quick in-person huddles. They are contained in this commit (edx@7b36805). Also, I renamed my placeholder modal to match the "visibility" term. That's fixed up in a previous commit now (edx@6b941fd) in case you want to cherry pick them both.

talbs · 2014-12-11T18:55:09Z

Thanks for the good messaging/copy feedback, @catong! I've made revisions in this commit (edx@4a74e27) based on your suggestions/notes.

talbs · 2014-12-11T21:09:25Z

@cahrens, here's the static build for the how I was hoping to flag any component with visibility explicitly set - a class that's added to the overall component/xblock wrapper. Shout if this is confusing or just plain wrong.

@talbs Since "visibility" is pretty overloaded (current "visibility" states are related to publish state, locking, etc.), can we make it explicitly about content groups? Perhaps "has-content-group-visibility-set"?

@cahrens, happy to scope this down a bit, but how does .has-group-visibility-set sound? I'd like to keep it a bit more generic group focused knowing that we'll ideally be adding in other types of groups to the modal that relates to this down the road (i.e. Content Experiment Groups),

Make sense and seem cool to you?

i like it! has-group-visibility-set seems solid and reusable.

frrrances · 2014-12-16T13:45:44Z

Nice work, @talbs! I have a few small comments but overall, really good stuff. Once you address my comments, I'm all 👍

talbs · 2014-12-16T21:49:57Z

@frrrances, I've addressed all of the very good feedback you gave. There's one note I made about _forms.scss abstraction. Let me know if that's okay by you and thanks for the look through.

@andy-armstrong and @explorerleslie, heads up that I corrected the extra visual space between two removed content groups in the modal. @andy-armstrong, I did have to make one small HTML/DOM change to do this. Here's the commit (edx@35be936).

Let me know if all of you are fine with these revisions. I'll squash them down to one "feedback-addressing" commit for easier cherrypicking.

frrrances · 2014-12-17T14:44:36Z

RTL @include margin-right($baseline/2);

talbs · 2014-12-17T15:09:22Z

Thanks for the RTL wisdom, @frrrances. I've made all of those conversions to make sure this UI is BFFs with RTL (commit is here if you wanna check - edx@af3659e).

* removing rem unit of measurement * removing not used visibility modal styling * simplifying form styling to rely on base form inheritance * removing extra padding from no-content UI * resolving extra padding for removed content group checkbox fields * RTLing ALLTHETHINGS

talbs · 2014-12-17T17:59:44Z

@andy-armstrong, I've gotten the famous @frrrances 👍 on this and rebased all of my additional feedback/fix commits into one for you to cherry-pick into your work (edx@ac507cb). Mind doing that and I'll close this PR?

talbs · 2014-12-17T19:18:07Z

Closing this since all of the UX/FED has been reviewed here and the appropriate commits now live on with https://github.com/edx/edx-platform/pull/6227.

talbs force-pushed the talbs/studio-component+groups-ui branch from 2240f15 to b4d620e Compare December 1, 2014 17:30

talbs force-pushed the talbs/studio-component+groups-ui branch 5 times, most recently from 91c045d to 75d7531 Compare December 10, 2014 00:13

talbs reviewed Dec 10, 2014
View reviewed changes

talbs force-pushed the talbs/studio-component+groups-ui branch 2 times, most recently from 308c867 to d5463a2 Compare December 10, 2014 00:32

talbs reviewed Dec 10, 2014
View reviewed changes

downzer0 reviewed Dec 10, 2014
View reviewed changes

Studio: adding UI and styling for component-level access modal

522c875

talbs force-pushed the talbs/studio-component+groups-ui branch from d5463a2 to 22422d3 Compare December 10, 2014 17:09

catong reviewed Dec 10, 2014
View reviewed changes

Comment thread cms/templates/js/publish-xblock.underscore Outdated

catong Dec 10, 2014

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: visible

andy-armstrong mentioned this pull request Dec 11, 2014

WIP - Studio support for cohorted courseware #6220

Closed

fixup! copy/messaging changes from wise docs folks

4a74e27

talbs reviewed Dec 11, 2014
View reviewed changes

frrrances reviewed Dec 17, 2014
View reviewed changes

Comment thread cms/static/sass/elements/_modal-window.scss Outdated

frrrances Dec 17, 2014

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RTL @include margin-right($baseline/2);

talbs force-pushed the talbs/studio-component+groups-ui branch from 600a843 to af3659e Compare December 17, 2014 15:08

talbs force-pushed the talbs/studio-component+groups-ui branch from af3659e to ac507cb Compare December 17, 2014 17:57

talbs closed this Dec 17, 2014

talbs deleted the talbs/studio-component+groups-ui branch December 17, 2014 19:18

Uh oh!

Conversation

talbs commented Dec 1, 2014

Uh oh!

explorerleslie commented Dec 2, 2014

Uh oh!

talbs commented Dec 3, 2014

Uh oh!

talbs commented Dec 10, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

downzer0 commented Dec 10, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talbs commented Dec 10, 2014

Uh oh!

talbs commented Dec 10, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talbs commented Dec 11, 2014

Uh oh!

talbs commented Dec 11, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frrrances commented Dec 16, 2014

Uh oh!

talbs commented Dec 16, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talbs commented Dec 17, 2014

Uh oh!

talbs commented Dec 17, 2014

Uh oh!

talbs commented Dec 17, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants