benetech / MathShare

MIT License
9 stars 3 forks source link

Home -> NewProblemsForm: Problematic Custom Rendering of Tabular Data #767

Open johnhbenetech opened 5 years ago

johnhbenetech commented 5 years ago

The dialog to add a new problem also shows the existing problems In the same set. The reason for that isn’t entirely clear, but the rendering of those existing problems is truly problematic for screen reader users.

Visually, the data is styled as a table, with CSS classes for rows and cells. But the markup only uses < div> elements (and < h5> elements for column headers). This means that:

There are other concerns which relate to general table accessibility, rather than specifically the custom mark-up pattern implemented here:

Aha! Link: https://diagramlabs.aha.io/features/B-129

johnhbenetech commented 5 years ago

Suggested Solution

It’s not known why the decision was made to use a custom table implementation pattern, but it should be avoided in all instances. First, properly mark up this data as a table. This means:

• Using a < table> element as the container for the whole thing, along with < tr> elements for each row and < td> for each cell. • Using < th> elements for row and column headers, and applying a scope attribute to those headers with a value of either “row” or “col”. Here, the current < h5> elements should act as column headers, while the position of each problem within the set should act as a row header.

Next, the accessibility of the data itself should be improved. The first column header should contain text such as “Position in Set”, rather than just the pound character. A fourth column should be added, with header text such as “Controls”, and the movement and image controls re-positioned into that column.

Finally, the table should be given an accessible description, using a element just inside the opening < table> tag. The text of the caption can read something like: “Current problems in this set”.

johnhbenetech commented 5 years ago

Proposed Code Sample

File: src/components/Home/components/NewProblemsForm/index.js

Note: this code sample does not include accessibility improvements for the problem controls, which are covered in a different issue

const problems = this.state.problems.map((problem, i) => {  …  return (   <tr key={i}>    <th scope="row">     {i + 1}     .    </th>    <td>     {`$$${problem.text}$$`}    </td>    <td>     {`$$${parseMathLive(problem.title)}}$$`}    </td>    <td>     {img}     <div className={styles.positionButtons}>      {moveUpBtn}      {moveDownBtn}     </div>    </td>   </tr>  ); }); 

const header = this.props.editing ? null  : (   <table>    <caption>     {Locales.strings.current_problems}    </caption>    <tr>     <th scope="col">      {Locales.strings.hash}     </th>
<th scope="col">      {Locales.strings.equation}     </th>     <th scope="col">      {Locales.strings.title}     </th>     <th scope="col">      {Locales.strings.problem_row_controls}     </th>    </tr>  ); 

const tableFooter = this.props.editing ? null  : (   </table>  ); 

return (  <AriaModal   …  >   <div className={styles.container} id="container">    {header}    {problems}    {tableFooter}    <MyWork     …    />    <div id="myWorkFooter" className={styles.footer}>     …    </div>   </div>  </AriaModal> ); 
jscholes commented 4 years ago

@johnhbenetech, @benetechMatt and @alexrcabral - there are still some issues with the table in this dialog (some of which relate to #764). Before enumerating the remaining issues, I do want to understand the thinking behind including the existing list of problems in a set, when the purpose of the dialog is to add a completely new one.

It could be that this is visually obvious, or I'm missing some important aspect of the product design. But it severely increases the complexity and amount of information in the modal.

My assumption is that the thinking goes something like:

  1. User adds new problem to set.
  2. User decides that their new problem shouldn't be at the bottom, so moves it in the table, without having to close the modal.
  3. User adds additional problem.
  4. Repeat until "Done" button is pressed.

If the above is true, and the ability to re-order problems is key to this dialog's functionality, then the label of the modal and its trigger are not very accurate right now. But I do think there is some unclear duplication going on here. For instance, if the user adds a problem and then wants to remove it, they have to close the modal anyway. Either this is the "Add New Problem" dialog, or a general-purpose "Edit Problem Set" dialog.

johnhbenetech commented 4 years ago

@benetechMatt I think I brought this up in the last week (from the UX side rather than a11y side).

I'll leave the decision up to you. We need to decide which of these views we want and where do they afford themselves to the user. This needs to be considered along with #816

jscholes commented 4 years ago

@johnhbenetech and @benetechMatt - going further, I have some serious concerns about having a fully fledged math editor inside a modal. It represents complex, interactive content being placed inside a dialog which is super easy to accidentally dismiss. Given how many actions can be performed within this context, it also breaks a user's expectation that a modal will be used for a single, bounded transaction and then closed.

How do you feel about this alternative flow:

  1. User clicks "Add New Problem".
  2. Modal appears, with just four elements:
    • Level 1 heading: "Add New Problem to [set name]"
    • Title text field
    • OK button
    • Cancel button
  3. User types a title and clicks OK.
  4. Modal closes, and application redirects user to the standard page with the math editor.

After step 2, they could also click Cancel, which would just take them back to the problem list.

benetechMatt commented 4 years ago

The ability to quickly add multiple problems without clicking "add new problem" in the problem list is a nice time saver. Thinking to the future we'll likely add the ability to add a correct answer, hints, etc. The problem set page may also change in the future to allow teachers to view student responses and statistics related to that set in addition to the problems in it.

I'm thinking it would make more sense to take users to a new page to add problems whether they just created the set or they are editing problems.

  1. create new set [on dashboard]
  2. taken to new set page. click on "add problem"
  3. taken to new page for adding/editing problems that is the same as the current modal
  4. user clicks "done" at bottom and taken to the problem list

clicking edit or add problem takes teachers to the same problem editing page as in step 2

@alexrcabral thoughts on the UX for this? @jscholes would this flow be easier for users to follow?

jscholes commented 4 years ago

@benetechMatt yes, I like that a lot.

benetechMatt commented 4 years ago

@alexrcabral let's discuss this in the next UX meeting

jscholes commented 4 years ago

@benetechMatt and @alexrcabral - have you discussed this UX going forward? I don't want to make further recommendations related to the modal if it will be replaced with a full page (which as indicated I think is a great idea).

jscholes commented 4 years ago

@benetechMatt and @alexrcabral - Just following up on this, because other tickets such as https://github.com/benetech/MathShare/issues/770 also depend on it.

benetechMatt commented 4 years ago

@jscholes, @alexrcabral and I will need to revisit this in a week

benetechMatt commented 4 years ago

@alexrcabral has this been brought up in the new mobile UI? Do we want to move forward with this or wait for the new UI?

benetechMatt commented 4 years ago

per conversation with @alexrcabral, we'll wait for a potential redesign of the add problem process.

alexrcabral commented 4 years ago

Adding some extra info: This is being addressed as part of the redesign and is currently in the process of being designed. When this is implemented, it should take into account this piece of using standard table elements, ideally on its own page rather than contained within a modal.