Skip to content

Presentation updates, part 2#29

Merged
e-kolpakov merged 7 commits intoedx-releasefrom
presentation-updates
May 7, 2015
Merged

Presentation updates, part 2#29
e-kolpakov merged 7 commits intoedx-releasefrom
presentation-updates

Conversation

@e-kolpakov
Copy link
Contributor

Fixes and improvements:

  • Give all the tables of the dashboard the same width (should be ~700px to give enough place to display long titles), and center the tables
  • Increase the width of the color codes in the table, to make them squarred instead of rectangular
  • Make the label for the "Average" row in the tables semi-bold (font-weight: 600)
  • Change aria-labels to off-screen text, as per Mark Sadeki's accessibility review: Presentation updates #16 (comment)
  • In the export HTML:
    ** Use left alignment for text within the tables, which is currently shown centered<
    ** Apply the changes done to the table in the LMS: center the tables, use the same width for all tables (~700px - do not make it stretch to all available space, as widows can be big and it will look ugly), make the color codes squared, make the label for the "Average" row squarred
  • In Problem Builder, allow to customize the title for the Messages, which by default reads "Feedback"

Sandbox: LMS, Studio
JIRA Ticket: OSPR-575
Merge deadline: preferably Friday, May 8 - this fixes are needed for course starting very soon.

Test instructions:

  1. Set up a couple of mentoring XBlocks.
  2. Set up Dashboard block to draw data from those blocks
  3. Set custom value for "Feedback Header" for one of mentoring XBlocks, add completion message to this XBlock
  4. Publish everything
  5. Go to LMS, provide answers for mentoring blocks. Observe that overriding feedback messages header works - when mentoring with custom "Feedback Header" and "Completion message" is submitted, both configured header and message is shown (If "Feedback" is shown instead of overridden value - it is a bug).
  6. Navigate to dashboard block

Observe the following:

  • All tables are centered and have 700px width
  • "Average" row caption is semibold (font-weight: 600)
  • Color-codes are much wider (30px) - should be square if question text fit to one line
  • Clicking "Download Report" (surprisingly) downloads a report. The report contains a copy of dashboard block visual representation and tables. All observations above applies to report content (table width, color codes and semi-bold "Average"). In addition, text in tables is left-aligned (it used to be centered).

@e-kolpakov e-kolpakov closed this May 5, 2015
@e-kolpakov e-kolpakov reopened this May 5, 2015
Copy link

Choose a reason for hiding this comment

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

perhaps "No score yet"? "value" isn't as specific ("value for what?") whereas "score" or "problem score" is much more specific.

@sarina
Copy link

sarina commented May 5, 2015

This is looking good to me. Please ping when there's a sandbox and I'll take a look there, and have Mark review your sr additions.

@e-kolpakov e-kolpakov force-pushed the presentation-updates branch from f896b96 to 3e35510 Compare May 6, 2015 08:55
Copy link
Member

Choose a reason for hiding this comment

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

Why 1.875? If the goal is to make them square, shouldn't it be 2? 2 certainly looks more square when I try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Kelketek cell height is 21.6 + 2_3.2 (padding) + 2_1 (border) = 30. Font-size or line-height, or whatever is equal to 1em for the cell is 16px; 30/16 = 1.875;

I've started with 2em and doesn't look more square to me :)

UPD: I've checked it out in sandbox; body height is 23.6 now, so 2em is more square. But it definitely was 21.6 when I developed it; maybe LMS styles have changed recently - I'll update it.

@Kelketek
Copy link
Member

Kelketek commented May 6, 2015

@e-kolpakov Please consider the note I put about the border width, but 👍 . @sarina Links to the sandbox have been added in the PR description.

@sarina
Copy link

sarina commented May 6, 2015

@cptvitamin can you take a look at the a11y changes here? (you reviewed #16 a few days ago, this is the response)

@e-kolpakov e-kolpakov force-pushed the presentation-updates branch from 19251ea to 45b3609 Compare May 6, 2015 15:02
Copy link

Choose a reason for hiding this comment

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

You might want to just consider defining some baseline variables then having sizes be relative to that (eg baseline/4, baseline/10 or whatever). That way you don't have to do fussy math all the time and worry that numbers like 1.875 are correct.

Perhaps a future improvement to think about.

@cptvitamin
Copy link

👍 from me.

@sarina
Copy link

sarina commented May 6, 2015

👍

e-kolpakov added a commit that referenced this pull request May 7, 2015
@e-kolpakov e-kolpakov merged commit 2b6e570 into edx-release May 7, 2015
@e-kolpakov e-kolpakov deleted the presentation-updates branch October 22, 2015 09:47
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