Skip to content

Override layout of choice label in Apros theme - #141

Merged
itsjeyd merged 1 commit into
masterfrom
long-choices-width
Feb 10, 2017
Merged

Override layout of choice label in Apros theme#141
itsjeyd merged 1 commit into
masterfrom
long-choices-width

Conversation

@itsjeyd

@itsjeyd itsjeyd commented Feb 9, 2017

Copy link
Copy Markdown
Member

... to incorporate feedback from QA team (cf. #139 (comment)).

Follows up on #139.

Screenshots: Apros

Problem Builder:

problem-builder-apros

Step Builder:

step-builder-apros

Screenshots: LMS

Problem Builder:

problem-builder-lms

Step Builder:

step-builder-lms

Test instructions

  1. Make sure you're using the Apros theme. In lms.env.json:

    "XBLOCK_SETTINGS": {
        "mentoring": {
            "theme": {
                "package": "problem_builder",
                "locations": ["public/themes/apros.css"]
            },
  2. Add MCQs and MRQs with long choice texts to a Problem Builder block.

  3. Access Problem Builder block in Apros. Observe that layout of choices matches screenshot above.

  4. Repeat previous steps for a Step Builder block.

  5. Switch to LMS theme:

    "XBLOCK_SETTINGS": {
        "mentoring": {
            "theme": {
                "package": "problem_builder",
                "locations": ["public/themes/lms.css"]
            },
  6. Access Problem Builder and Step Builder blocks in the LMS. Observe that layout of choices matches screenshots above.

Notes

Choice tips will partially cover long choice texts when using the Apros theme. This is not a regression as far as McKinsey is concerned: When switching back to 484131c (parent of merge commit from #125), we get the same behavior. Also, PRs following #125 did not introduce further changes to the layout of choice texts and choice tips.

Reviewers

@mtyaka

mtyaka commented Feb 10, 2017

Copy link
Copy Markdown
Member

👍 I tested this in apros - it looks good, just like the screenshots.

  • I tested this: Tested the new changes in apros
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation

@itsjeyd

itsjeyd commented Feb 10, 2017

Copy link
Copy Markdown
Member Author

Thanks @mtyaka! I'll follow up with a version bump PR later on today.

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.

2 participants