Skip to content

Comprehensive Theming - #8271

Merged
singingwolfboy merged 15 commits into
masterfrom
sass-theming
Sep 21, 2015
Merged

Comprehensive Theming#8271
singingwolfboy merged 15 commits into
masterfrom
sass-theming

Conversation

@nedbat

@nedbat nedbat commented May 29, 2015

Copy link
Copy Markdown
Contributor

Acceptance criteria:

  • choose what sass variables will get !default
  • all existing tests must pass
  • docs: describe how to change your old-style theme to still work. [NB]
  • docs: describe how to make a new-style theme. [DB]
  • name this theming system (maybe). "Comprehensive theming", with "comp_theme" for settings.
  • decide on the settings naming (see https://github.com/edx/configuration/pull/2075).
  • decide whether to ship a non-edX theme. (We are shipping at least one theme to support testing)
  • decide whether COMP_THEME_DIR needs to be set. (COMP_THEME_DIR must be set, but can be None, as common sets it.)
  • testing: overriding templates works.
  • test a complex microsite.
  • test a stanford theme.
  • fix it so microsites still work without renaming footer.html to default-footer.html [DB]
  • i18n collection working
  • choose reviewers (cdodge, ux, tusbar, stanford person?)

More things to do:

  • Are we continuing to support Stanford themes, or are we saying they must be converted?
  • Revisit why we have "header.html" but "themable-footer.html". Can we make them more symmetric? (See comment in the review)
  • Chase down the filenames that have been obsoleted that themers need to know about.
  • "These changes will have to be ported to django_main.html" (see comment below.)
  • Make sure we can theme certificates: certificates/sass/main-ltr.scss, etc.
  • red-theme should re-style more things to be more completely red
  • red-theme .scss file should have paragraph explaining how to find things to override.

@singingwolfboy
singingwolfboy force-pushed the sass-theming branch 7 times, most recently from 3e4d129 to 3a09def Compare June 4, 2015 15:29
nedbat
nedbat reviewed Jun 5, 2015
View reviewed changes
Comment thread lms/templates/courseware/courses.html Outdated

@nedbat nedbat Jun 5, 2015

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.

We'll need to keep this for microsites to keep working.

@singingwolfboy
singingwolfboy force-pushed the sass-theming branch 3 times, most recently from a27aa6b to 240a7ca Compare June 12, 2015 13:42
@nedbat nedbat mentioned this pull request Jun 17, 2015
Merged
@singingwolfboy
singingwolfboy force-pushed the sass-theming branch 5 times, most recently from 5c5a3a5 to fb6819f Compare July 9, 2015 14:39
@nedbat
nedbat force-pushed the sass-theming branch 2 times, most recently from c33c05b to 51998b4 Compare July 27, 2015 20:53
@nedbat
nedbat force-pushed the sass-theming branch 4 times, most recently from d166d48 to fb7233f Compare August 6, 2015 17:34
singingwolfboy added a commit that referenced this pull request Sep 21, 2015
@singingwolfboy
Merge pull request #8271 from edx/sass-theming
03aeae4 
Comprehensive Theming
@singingwolfboy
singingwolfboy merged commit 03aeae4 into master Sep 21, 2015
@sarina

sarina commented Sep 21, 2015

Copy link
Copy Markdown
Contributor

WHATTTTT

dance-party

OMGGGGGG

dance-parttttyyyy

YAYAYYAYAYA

pika

@talbs

talbs commented Sep 21, 2015

Copy link
Copy Markdown
Contributor

Hmmm, what happened to some of the TODOs that @nedbat noted as well as any open discussion points? Are those already taken care of are they carrying on to another branch/PR as a follow-up?

@singingwolfboy

singingwolfboy commented Sep 22, 2015

Copy link
Copy Markdown
Contributor

@talbs I figured that we could handle most of those TODOs as follow-up pull requests, rather than further delaying the merge of this PR. There some questions that I can answer, though:

  • We're no longer supporting Stanford themes -- Stanford's repo has diverged widely from edX's repo, and they're the people who can best provide support for it. Instead, we'll be asking people to convert their Stanford theme over to the new comprehensive theming system.
  • The certificates repo will need to be modified to support comprehensive theming, but the structure and layout of a comprehensive theme already has a place for theming assets for certificates. Modifying the certificates repo is follow-on work.
  • My next task on this is writing documentation and making a pull request into the edx-documentation repo. This documentation will include information about "header.html" vs "themable-footer.html", and how to find what Sass variables to override.

@talbs

talbs commented Sep 22, 2015

Copy link
Copy Markdown
Contributor

Thanks for the 4-1-1, @singingwolfboy. Very helpful.

Let me know what I can do to support things and I have a few UX-related tasks to:

  • help flesh out the demo theme a bit more
  • figure out how to update pattern library assets, compiles, and standards to mimic this new organization as well.

I'll keep you posted on those bits.

@talbs

talbs commented Sep 22, 2015

Copy link
Copy Markdown
Contributor

@singingwolfboy, after talking with @ebporter, there are a few questions around the following:

We're no longer supporting Stanford Theming -- Stanford's repo has diverged widely from edX's repo, and they're the people who can best provide support for it. Instead, we'll be asking people to convert their Stanford theme over to the new comprehensive theming system.

  • Do we know how many folks/partners have implemented the Stanford way of theming on current instances?
  • Is there a plan to create any guidelines or help in converting an instance from Stanford Theming to the new Comprehensive Theming?
  • Is there any partner-to-partner communication planned to tell Stanford of this decision

Happy to follow any standards or protocols you guys have for larger feature/mechanic changes in general.

@singingwolfboy

singingwolfboy commented Sep 22, 2015

Copy link
Copy Markdown
Contributor

@talbs: @stvstnfrd from Stanford took a look over the pull request, and didn't raise any major issues with it. (@stvstnfrd, if I'm misinterpreting your comments, please let me know.) The themes/README.rst file that was introduced in this pull request contains some preliminary documentation about this feature, including how to alter Stanford themes to work correctly. I'm currently working with the Documentation team at edX to transform this file into proper documentation to go up on readthedocs.org, along with the rest of our documentation.

@ebporter

ebporter commented Sep 23, 2015

Copy link
Copy Markdown

Re-asking the question @singingwolfboy :

Do we know how many folks/partners have implemented the Stanford
way of theming on current instances?

Sounds like we've talked with @stvstnfrd and he is okay with this; but there are others who are using the Stanford theme, right? So, what kind of outreach do you think we may need for that group?

@tusbar

tusbar commented Sep 23, 2015

Copy link
Copy Markdown
Contributor

I would say that about 30 organizations are using themes based on the IONISx theme. Those people may not be aware that their theme is a Stanford style theme.

I am fine with the change, and I will make sure that the base IONISx theme is updated accordingly.

@ebporter

ebporter commented Sep 23, 2015

Copy link
Copy Markdown

Thanks @tusbar !

@singingwolfboy singingwolfboy mentioned this pull request Sep 29, 2015
Merged
singingwolfboy added a commit that referenced this pull request Sep 29, 2015
@singingwolfboy
Remove the edx.org comprehensive theme
8ab401d 
There's a lot of useful things in PR #8271 that provide a framework
for the comprehensive theming system. If we need to remove the edx.org
theme from the codebase, we can leave most of PR #8271 in, to make it
easier to build on top of and get this feature back in.
singingwolfboy added a commit that referenced this pull request Sep 30, 2015
@singingwolfboy
Remove the edx.org comprehensive theme
116ca29 
There's a lot of useful things in PR #8271 that provide a framework
for the comprehensive theming system. If we need to remove the edx.org
theme from the codebase, we can leave most of PR #8271 in, to make it
easier to build on top of and get this feature back in.
singingwolfboy added a commit that referenced this pull request Sep 30, 2015
@singingwolfboy
Remove the edx.org comprehensive theme
aa8ecca 
There's a lot of useful things in PR #8271 that provide a framework
for the comprehensive theming system. If we need to remove the edx.org
theme from the codebase, we can leave most of PR #8271 in, to make it
easier to build on top of and get this feature back in.
@singingwolfboy
singingwolfboy deleted the sass-theming branch October 7, 2015 19:06
@singingwolfboy singingwolfboy mentioned this pull request Oct 7, 2015
Merged
singingwolfboy added a commit that referenced this pull request Oct 9, 2015
@singingwolfboy @macdiesel
Remove the edx.org comprehensive theme
8d55e32 
There's a lot of useful things in PR #8271 that provide a framework
for the comprehensive theming system. If we need to remove the edx.org
theme from the codebase, we can leave most of PR #8271 in, to make it
easier to build on top of and get this feature back in.
aamir-khan pushed a commit that referenced this pull request Oct 9, 2015
aamir-khan @macdiesel
Upgrade Django to v1.8.4
b34c380 
Fix of Flaky test in proctored exams.

CRI-40 Don't assume user in request context when grading

Test student_grade_summary_data method

MA-1182 - added test for not enrolled in course, removed commented out nose test code, formatted import statements according to edx standards

fixed the allowances table css issue in instructor dashboard

Upgrade coverage.py to 4.0b3

Fetch people.yaml file from private repo

If cannot fetch people.yaml, entire script should fail

Fixing bad button style

Removing unneeded styling on confirmation button; making the
appearance and hover/focus behavior of the button match that
of the confirmation dialog in studio.

TNL-3189

Add logic to handle not searching when focus is passed to the clear search button

TNL-3238

Fix the inheritance of tests into tests for the recommender tests

Remove trailing whitespace from Sass files

Right align text above action buttons

TNL-3234

Refactor the test_crud file.
Move Split-specific tests to test_split_modulestore.py.
Derive remaining tests from ModuleStoreTestCase.
Remove unused persistent_factory.py.

added wait for loading js files

Make studio forgot password link focusable

Removing extraneous IE search clear button

Taken from http://stackoverflow.com/questions/14007655/remove-ie10s-clear-field-x-button-on-certain-inputs.
Not yet tested, but will be soon.

Watch and process JS and CSS xmodule assets

Added messages after assets process steps

Allows username and user email to be sent to an LTI.

Currently, there is a way to do so via opening the LTI tool in a new
page, this removes the one line of code that specifies that it is the
only way to do it. It allows the iframe’d version of the tool to be
able to receive such information if necessary.

Internationalize course re-run strings

Break translators comments across multiple lines

Re-word translators comment

Fix Mathjax rendering bug

Remove membership field from teams when serializing memberships.

TNL-3281

Fix Translator comment syntax in Mako templates [TNL-3277]

Rebalance. Shard_5 is getting long comparatively.

Add management command for granting superuser access

Mako comments have to be double-hash, or they are included on the page.

moved variables from template to view

fixed broken tests and quality violations

moved twitter_url to view also

escaped string for js

Built a better course enrollment admin

CSS Animations disabled for test

Pin coveralls to specific version.

The latest version is compatible with the new coverage.py.

Optimize OpenID Course Claims for users.
- Course overviews will cache courses upon publish.
- Added management command to warm up cache.
- OAuth2 handler returns courses via course overviews.

MA-1149 fixed vote bug in discussion api

An incorrect response was being returned when voting for a comment
or thread.

Fix order of tracking backends

Add in a comment describing the reasoning for the names

Revert "Fix order of tracking backends"

Remove Tender and change Studio help links.

TNL-2695

Add Peer Instruction XBlock

Added error message in enrollment tab ccx coach dashboard

Emit event when last_activity_at is updated.

TNL-3303

Readonly for enrollment end date/time on edX (for non-global-staff)

TNL-2694

Add accessibility aria-readonly attributes

Added my name in authors

Update translations (autogenerated message)

Update verified track message.

ECOM-2300

Adding trailing_text to formulaequationinput

Add requirements files that can be used with package monitorig tool.

Assigning focus to the selected topic

TNL-2620

escape string literal also

SOL-1224

scroll to top after saving

fixed broken bok choy tests

SOL-1188

Remove second tooltip from LMS Sequentials

Remove blank lines from tooltip display

Comprehensive theming

This is a squash of 38 commits ending with
5b080f979d692804452400ac5bed9b17c50b001e

Use the proper 'off' default for COMP_THEME_DIR.

Minor changes, responding to pull request comments

Use sass interpolation instead of concatenation for -path

Comprehensive Theming: re-organizing/minor simplifying of existing LMS Sass variables

Comprehensive Theming: setting sass !default flag on themeable LMS base variables

Rename google_analytics.html to use a hyphen instead.

Document disparity between footer.html and themable-footer.html

Fix mako argument syntax

Remove unneeded .write() method

A test for optional_include_mako

Import the correct scss files for xmodule styling

fixup! simplifying needed sass assets for xmodule sass compilation

Use coverage.py 4.0 final, and coveralls to match

fix label for check/final check

This label hadn't been updated to change its message depending on whether students were on their "final" submission attempt.

Document child order in course structure api docs

Corrected read-only field display for CourseEnrollment admin

Read-only fields are read-only for existing models. These values can be set when creating new models.

ECOM-2172

Using a better admin interface for UserProfiles

This interface should actually be useful as it doesn't attempt to load all users on the edit page.

ECOM-2172

Updated admin interfaces for credit models

ECOM-2172

MA-1337 xBlock Rendering View

Add the ability to change the SESSION_SAVE_EVERY_REQUEST django setting

MA-1305 - formatted author label to Staff & Community TA

refactor community_ta and staff in docstrings

added response count in thread

MA-1212 - added response count field in tests

removed unneccessary function

"MA-1212 - added response count in thread retrieval service and updated tests"

updated serializerz test for response count

MA-1212

exclude response count from thread get list

removed response_count field from update and create thread services

made resp_total dynamic for cs thread in test_serielizer

quality fixed

improved quality

add TODO JIRA ticket on removed fields

SOL-1188 fix

fixed bug in certificate html view

Fixed bug: When certificate id previewed by a user who already has a
certificate generated with different mode it would not display
certificate in right preview mode.

Allow staff users to request a list of courses a student is enrolled in.

The list only includes those courses the requesting user has staff access for.

Optimise getting list of courses from Enrollment API for global staff.

Clarify docstring of the enrollment list endpoint of the Enrollment API.

TNL-3197 Partial Credit (not Dropdown)

Updated templates

Updating responsetype tests.

Updating other tests for PC

Putting OptionInput back the way it was.

Acceptance tests for partial credit

Testing tools

pep8

scss name fix

Missing }

icon fix

icon fix

Add mailchimp list fields and config for new users

More descriptive dashboard title

LMS: course navigation menu refactor

Upgrade to django-pipeline v.1.5.3.

TNL-3137

Use django.contrib.staticfiles instead of django-staticfiles.

TNL-3137

Revert "Upgrade django-pipeline and staticfiles"

Upgrade to the latest version of requests.

Changelog: https://pypi.python.org/pypi/requests

use edx-proctoring 0.9.6d

In cohort courses, student progress pages differ from the grade report.

TNL-3204

bump edx-proctoring 0.9.6d

A faster way to clean the .pyc files

On my devstack, the old line takes about 5.5 seconds, the new line takes
about 1.3 seconds.

For background:
http://nedbatchelder.com/blog/201505/be_careful_deleting_files_around_git.html

Remove "responsive" check before deciding whether to add a viewport tag.

We also need to ensure that this tag is presented when loading xblocks
via the mobile app anyway. Since we're fine just giving responsive
content to everyone we don't need this check any more.

JIRA: https://openedx.atlassian.net/browse/MA-960?filter=13805

Improve team testing of routers

Improve response handling in AjaxHelpers

I've changed the logic so that AjaxHelpers keeps
track of which requests have not yet had mock
responses sent. This ensures that every response
is handled before moving on to the next one,
rather than always handling the last request.
My intention is that this won't allow bugs to creep
in where a request isn't fired and instead the test
responds to an old request. It also should ensure
that extra events aren't accidentally fired.

date format "Monday at 3pm UTC" added for courses starting with-in 5 days

i18n fix

1. Add missing '_()' in capa_base.py & video_xfields.py
2. Fix a bug in instructor_dashboard.py that causes crash when ANALYTICS_DASHBOARD_NAME contains non-ascii characters.
3. Workaround for mis-extractions inside .underscore files when using gettext along with interpolate.

LMS: increasing contrast in gray meta/date text on course dashboard

LMS: increasing contrast of footer text

Disable button while running

Now using withDisabledElement to prevent multiple event handling. I've
attempted to write this as a common function in view_utils, so that this
approach can be used more generally, wherever needed. Includes a unit test.

Thanks to Christina and Andy for all their help on this one.

TE-1042 include diff-cover for js.

Note that because js testing occurs on a different CI job than python
unit testing, files will not be clobbered by this change. The python unit
testing job also creates the same-named report through the diff_coverage
task.

Use correct reports for jshint diff-quality.

LMS: a11y contrast fixes for find course page

Add main index api doc label for inter sphinx

Update sinon to version 1.17.0

LMS: replacing proctoring markup from accordion

Initial implementation of API for listing a user's third party auth providers

Manage focus on modal confirmation prompt

make verifyElementInFocus a ViewHelpers method

edx-procoting will be passing back which font-awesome icon to use

prevent delete public oprhans

Discussion forum styling cleanup to correct current visual styling issues in addition to cleaning up styles in preparation  for adding post images

updated  error message

image stretched issue resolved in signatories

override wrapper-msg styles for auto-cert

fixed bug in certificate html view

Fixed bug: When certificate id previewed by a user who already has a
certificate generated with different mode it would not display
certificate in right preview mode.

bump version of edx-proctoring to 0.9.11

Upgrade djangorestframework to v3.1

* Upgrade edx-submissions
* Upgrade edx-ora2
* Upgrade edx-val
* Upgrade edx-proctoring
* Update all edx-platform code that depends on DRF, including:
  - auth_exchange
  - cors_csrf
  - embargo
  - enrollment
  - util
  - commerce
  - course_structure
  - discussion_api
  - mobile_api
  - notifier_api
  - teams
  - credit
  - profile_images
  - user_api
  - lib/api (OAuth2 and pagination)

Fixes for rebasing of the release on 2015-09-23

Revert "Merge DRF 3.1 in to master"

reverting the chnages sent in MAYN-131, final solution will be submitted in a separate PR

Line breaks in signatory title issue fixed

Upgrade djangorestframework to v3.1

* Upgrade edx-submissions
* Upgrade edx-ora2
* Upgrade edx-val
* Upgrade edx-proctoring
* Update all edx-platform code that depends on DRF, including:
  - auth_exchange
  - cors_csrf
  - embargo
  - enrollment
  - util
  - commerce
  - course_structure
  - discussion_api
  - mobile_api
  - notifier_api
  - teams
  - credit
  - profile_images
  - user_api
  - lib/api (OAuth2 and pagination)

Fixes for rebasing of the release on 2015-09-23

added test to discussions_api client to test JSON response that is absent of the resp_total key

fixed PEP-8 violations

Bump version of segemnt's analytics-python package to 1.1.0

Wrap 3rd Party Auth Pipeline with Segment enabled checks

Replace SEGMENT_IO_LMS & SEGMENT_IO_LMS_KEY with single key SEGMENT_KEY

use platform_name template tag

Revert "Fix Mathjax rendering bug"

Fixed receipt page bug

Variable name was broken by a commit in #9868.

ECOM-2431

Remove the edx.org comprehensive theme

There's a lot of useful things in PR #8271 that provide a framework
for the comprehensive theming system. If we need to remove the edx.org
theme from the codebase, we can leave most of PR #8271 in, to make it
easier to build on top of and get this feature back in.

Disable some tests for comprehensive theming

Depends on functionality that has been temporarily reverted

update reference to edx-proctoring to account for requested iconography changes from UX team. Also update test to conform with changes

Revert "prevent delete public oprhans"

This reverts commit 3849a8f.

Rename SEGMENT_KEY to have a unique key name for both CMS and LMS

Multiple credit fixes

- Added support for Unicode characters when creating the signature
- Using <textarea> for form data to support newline characters
- Generating form with jQuery instead of relying on existing DOM element
- Removed duplicated credit request code
- Corrected spaing on dashboard
- Enabled functionality by default

Dashboard cleanup
-- Cleaned styles
-- Added wrapper class to use for hiding the actions
-- Cleaned JS toggling of error container
-- Corrected dashboard and receipt text

ECOM-2172

Limited grades to seven characters

ECOM-2433

add new column 'copy_id_photo_from' for 'SoftwareSecurePhotoVerification' model to track initial verification with 'photo_id_key'

Add management command to set SoftwareSecure verification status.

When retrying, use the previously copied photo_id

Allow retry_failed_photo_verifications to take a list of receipt_ids to retry

Fixed credit form submission bug

Firefox requires the form to be attached to a DOM element

ECOM-2492

Removed mailing address from provider request

Given the time crunch this is the safest option for dealing with newlines.

ECOM-2492

Credit fixes

- Fixed receipt page button
- Fixed message on dashboard

ECOM-2492
singingwolfboy added a commit that referenced this pull request Oct 9, 2015
@singingwolfboy @macdiesel
Remove the edx.org comprehensive theme
1dd7a76 
There's a lot of useful things in PR #8271 that provide a framework
for the comprehensive theming system. If we need to remove the edx.org
theme from the codebase, we can leave most of PR #8271 in, to make it
easier to build on top of and get this feature back in.
@bradenmacdonald bradenmacdonald mentioned this pull request Feb 26, 2016
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.