Skip to content

LMS: Student Notes - View All/Search Final Styling (UX-1331) - #6408

Closed
talbs wants to merge 20 commits into
feature/edxnotesfrom
talbs/edxnotes-notesview
Closed

LMS: Student Notes - View All/Search Final Styling (UX-1331)#6408
talbs wants to merge 20 commits into
feature/edxnotesfrom
talbs/edxnotes-notesview

Conversation

@talbs

@talbs talbs commented Dec 30, 2014

Copy link
Copy Markdown
Contributor

This work styles the various states of the aggregated notes view.


Known Concerns

  • I created this branch from https://github.com/edx/edx-platform/pull/6384, which contains a few base styles/utilities for this work. Once that PR is merged or cherry-picked, this should be rebased and can stand on its own.
  • I revised some existing class names that JS methods relied on (and tried to change the JS). @polesye or @tymofij, I need your help correcting tests and proofing the production JS

Screengrab - Latest Notes
screenshot-localhost 8000 2014-12-30 18-23-47

Screengrab - Notes by Course Structure
screenshot-localhost 8000 2014-12-30 18-24-57

Screengrab - Search No Results
screenshot-localhost 8000 2014-12-30 18-24-28

Screengrab - Search Blank Error
screenshot-localhost 8000 2014-12-30 18-25-41

@talbs
talbs force-pushed the talbs/edxnotes-notesview branch 5 times, most recently from 43a057f to ca5f16f Compare December 30, 2014 23:21
@talbs

talbs commented Dec 30, 2014

Copy link
Copy Markdown
Contributor Author

@clrux, @explorerleslie, and @polesye or @tymofij, here's a PR for https://openedx.atlassian.net/browse/UX-1331.

Note, I've made this work based off of my other open PR's commits. The one commit you should care about for this work is edx@ca5f16f


@frrrances, can you review my markup, styling and FED work here?

@polesye and @tymofij ,I've adjusted some markup/JS here that tests may need to be synced up with. Can you take a look to make sure the JS is still good and your tests work?

@explorerleslie, I've attached screengrabs to show states (until all of this work and the previous PR can be put on the feature sandbox). Feel free to review from a Product/overall UX PoV.

@clrux, no action needed, the changes here are just an FYI on what I had to do on the FED and visual design levels of our project.

@talbs talbs changed the title (WIP) LMS: Student Notes - View All/Search Final Styling (UX-1331) LMS: Student Notes - View All/Search Final Styling (UX-1331) Dec 30, 2014
@talbs

talbs commented Dec 31, 2014

Copy link
Copy Markdown
Contributor Author

@frrrances, don't taze me on the RTL stuff yet, bro! Getting a fixup! commit with that stuff in place shortly.

@talbs
talbs force-pushed the talbs/edxnotes-notesview branch from ca5f16f to 0ed67c1 Compare January 3, 2015 15:42
@talbs

talbs commented Jan 3, 2015

Copy link
Copy Markdown
Contributor Author

@frrrances, I've added in RTL support for this work and am hands off now until you're done with your review. Let me know if you need help setting up notes locally. This gist really helped with devstack bits, but there are a few more steps in Studio afterwards.

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.

@talbs I thought we're using 4 spaces for indentation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Apologies - moving this back to the land of quad-spacing!

@polesye

polesye commented Jan 5, 2015

Copy link
Copy Markdown
Contributor

Unit tests are fixed.

@explorerleslie

Copy link
Copy Markdown

looks good 👍

talbs added 5 commits January 5, 2015 16:10
* revising notes view styling scope to use a <body> class
* revising HTML basics: indentation levels, button UI classes, view-level styling and semantics
* basic Sass/CSS clean up
* deeper Sass/CSS clean up + abstraction
* typography syncing
* refining and defining notes visual color
* changing reference heading copy
* revising Student Notes visual styling
* revising no search results styling/UI
* revising search error styling/UI
@talbs
talbs force-pushed the talbs/edxnotes-notesview branch from f8b28dd to b9c73ec Compare January 5, 2015 21:21

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.

Is this latin going to get cleaned up before merge?

@frrrances

Copy link
Copy Markdown
Contributor

@talbs nice work! code looks good... i couldn't even find much to nit over! can we connect tomorrow so i can get my local set up for UX review?

@talbs

talbs commented Jan 6, 2015

Copy link
Copy Markdown
Contributor Author

@frrrances, firstly, I don't know what you have against the Latin language (its not dead, yet!). :P

Thanks for the review, I'll clean up those bits and am happy to help you get set up locally with notes. A few things that will help:

@talbs

talbs commented Jan 7, 2015

Copy link
Copy Markdown
Contributor Author

Closing this branch/PR in favor or a fresher one with cherry-picked commits - https://github.com/edx/edx-platform/pull/6471

@talbs talbs closed this Jan 7, 2015
@talbs
talbs deleted the talbs/edxnotes-notesview branch January 9, 2015 03:24
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.

5 participants