Skip to content

Conversation

@talbs
Copy link
Contributor

@talbs talbs commented Jul 17, 2014

This work adds styling support for section/subsection settings modal UI (needed for https://github.com/edx/edx-platform/pull/4425) and includes the following steps:

  • revising mark up/semantics for modal form UI
  • refactoring and revising styling for modal form UI
  • adding basic modal content support styling
  • changing section/subsection modal window size to 'large'
  • converting subsection grading type selection UI to native select element

@talbs
Copy link
Contributor Author

talbs commented Jul 17, 2014

@auraz and @andy-armstrong, here's the styling and UI work to support https://github.com/edx/edx-platform/pull/4425.

A few notes on this work:

  • I've significantly revised the HTML structure and class attributes of the .underscore template that renders this UI. I've tried to make sure that I didn't remove any classes that controlled functionality, but did remove classes that seemed extraneous or didn't fit the purpose of the UI/styling directly.
  • I'm not sure if this work causes issues with tests - @auraz, can you review and revise any tests that are needed
  • I've converted the subsection grade dropdown UI to use a native <select></select> form element instead of a custom UI (which was originally needed due to Course Outline layout). Since we're putting all of these controls in a dedicated, open space, this custom UI isn't needed anymore and actually causes more effort to maintain its style than its worth. @auraz and @andy-armstrong , can you guys implement this change (there's an example of the rendered HTML I'm expecting)?
  • I've duplicated some of the _forms.scss styling for use in modal window forms (and made a TODO: for UX to clean up in the near future).
  • On subsection settings modals, the clear control (.action-clear) used to reset/clear dates seems to be affecting both date pair fields rather than scoped to just the pair closest to the control. @auraz, can you correct this, please?

Here's a screenshot of what the UI looks like on this branch...
studio-edit-modal

@frrrances, can you review my work here from a UI, visual, and FED perspective?


FYI @cahrens and @explorerleslie

@tymofij
Copy link
Contributor

tymofij commented Jul 17, 2014

I have fixed functional things here and put up a sandbox http://tymofij.m.sandbox.edx.org/
IMO this is safe to be merged into alex/edit_course_outline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andy-armstrong, just wanted to confirm that these values for grading types should be hardcoded here. I originally added them into my static mockup with a note to replace/abstract. Should they be pulling from a course's individual assignment types?

Copy link
Contributor

Choose a reason for hiding this comment

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

They most definitely should not be hardcoded, and in later commits I fixed that.

@talbs
Copy link
Contributor Author

talbs commented Jul 17, 2014

@tymofij, thanks for the update.

I do have one concern about the grading types select menu having static values and have pinged @andy-armstrong to make sure that's expected.

I'm also waiting for @frrrances to review my styling work here.

After those are both resolved, I'm fine with this merging from my perspective.

@tymofij
Copy link
Contributor

tymofij commented Jul 18, 2014

@auraz @polesye - please take a look and let's merge this into our branch alex/edit_course_outline, which itself is still a work in progress. So that @frrrances and @andy-armstrong would have one less PR to review.

@auraz
Copy link
Contributor

auraz commented Jul 18, 2014

👍 Please rebase first!

@polesye
Copy link
Contributor

polesye commented Jul 18, 2014

👍

talbs and others added 6 commits July 18, 2014 16:21
* revising mark up/semantics for modal form UI
* refactoring and revising styling for modal form UI
* adding basic modal content support styling
* changing section/subsection modal window size to 'large'
* converting subsection grading type selection UI to native select element
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This <br /> element is not needed here for style or semantics. Would you mind removing it?

Copy link
Contributor

Choose a reason for hiding this comment

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

would you mind removing br?

Sure. It was actually a debug artifact.

tymofij added a commit that referenced this pull request Jul 18, 2014
Studio: Styling Support for Section/Subsection Settings Modal UI
@tymofij tymofij merged commit a350eae into alex/edit_course_outline Jul 18, 2014
@talbs
Copy link
Contributor Author

talbs commented Jul 18, 2014

@tymofij, I was waiting on @frrrances to review my UI and HTML/Sass changes on this branch. Since you merged it, @frrrances can you review my work (UI and FED) on https://github.com/edx/edx-platform/pull/4425?

@tymofij
Copy link
Contributor

tymofij commented Jul 18, 2014

@talbs Yes, that would be most productive course of actions.

@tymofij
Copy link
Contributor

tymofij commented Jul 18, 2014

Yes, that would be most productive.

On 18 July 2014 17:09, Brian Talbot notifications@github.com wrote:

@tymofij https://github.com/tymofij, I was waiting on @frrrances
https://github.com/frrrances to review my UI and HTML/Sass changes on
this branch. Since you merged, it, @frrrances
https://github.com/frrrances can you review my work (UI and FED) on
#4425 https://github.com/edx/edx-platform/pull/4425?


Reply to this email directly or view it on GitHub
https://github.com/edx/edx-platform/pull/4455#issuecomment-49434646.

Sincerely, Tim Babych
http://clear.com.ua

Copy link
Contributor

Choose a reason for hiding this comment

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

"CASE: short length" - not long length

@tymofij tymofij deleted the talbs/style-outline-modal branch July 23, 2014 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants