Improve Responsive Design in My Lists

Bug #1681943 reported by Terran McCanna
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Medium
Unassigned
2.11
Won't Fix
Undecided
Unassigned
2.12
Fix Released
Medium
Unassigned

Bug Description

I agree with Kathy's suggestion to pursue a better and comprehensive approach to all of the My Account pages in Bug #1623062 (https://bugs.launchpad.net/evergreen/+bug/1623062), but considering that it may take some time before any of us can tackle that, I have done some work on the My Lists page for an interim improvement to the responsive design.

Prior to this change, the list content ran off the side of the page triggering a sideways scroll bar and problems with readability, and the Actions dropdown for a list would disappear on a mobile view. Changes I've made include:

- Moved action dropdown out of header row so that it doesn't disappear when header row is hidden on a mobile device.
- Modified Temporary List to be more consistent with permanent lists in its use of CSS.
- Added a title to the section for editing the title and description.
- Hid most columns from mobile view, leaving just title and author.
- Modified the CSS for the list buttons so that they'll line up more neatly in both the responsive view and in the ordinary view.

I'll post a patch momentarily.

Revision history for this message
Terran McCanna (tmccanna) wrote :
tags: added: pullrequest
Revision history for this message
Terran McCanna (tmccanna) wrote :

Changes in place on http://terran-master.gapines.org with Concerto logins.

Galen Charlton (gmc)
Changed in evergreen:
status: New → Confirmed
importance: Undecided → Medium
importance: Medium → Wishlist
Revision history for this message
Galen Charlton (gmc) wrote :

Tested and signed off; branch is user/gmcharlt/lp1681943_signoff.

The signoff branch includes an additional style tweak. Terran, could you double-check that I resolved the merge conflict correctly?

tags: added: signedoff
Changed in evergreen:
milestone: none → 3.0-alpha
Revision history for this message
Terran McCanna (tmccanna) wrote :

Galen - looks good to me, thank you!

Revision history for this message
Dan Scott (denials) wrote :

I don't think this is ready to be pushed; the commit introduces a lot of new CSS that should also be reflected in the RTL styles (float:left, margin-left, etc).

On a lesser note, I'm worried about the approach of making things look better on mobile by taking away content. This can be frustrating to those who now have no option for seeing the content that is hidden. That said, I have not yet offered up an alternative design or implementation, so I won't object to the commit being pushed on this particular basis.

Revision history for this message
Kathy Lussier (klussier) wrote :

Adding a note that I would like this to be considered a bug fix since the current mobile view of My Lists is so broken. I haven't looked at the code yet, but my understanding is that it also fixes bug 1353509, which would be a nice fix to backport.

I'll add backport targets to this bug, but if anyone disagrees with me, feel free to let me know!

Terran, one trick I've used when adding styles to the RTL stylesheets is updating en_US in config.i18n_locale to be an RTL language. I find it easier to assess the work when I'm working in my own language than viewing the catalog in Arabic.

Revision history for this message
Terran McCanna (tmccanna) wrote :

Yes, it does fix bug 1353509 as well, thanks for catching that Kathy!

I am tied up in a conference this week and probably won't have time to work on testing RTL until next week.

Revision history for this message
Ben Shum (bshum) wrote :

The changes for master don't apply cleanly anymore now that we've merged the changes for RTL+LTR merging. Need to get a fresh rebase to master and adapted with the new styles for RTL/LTR switching. Also, need to decide how to apply this change for rel_2_12 and rel_2_11 if backported (as per current targeting), do we keep the original branch as is and not worry about the RTL handling? Or is this considered a "new feature" that we've enhanced the responsive design for my lists in future release (3.0) and thus not backported?

My vote is that if it's new and better, which it seems like it is, that could be considered new feature-ish and thus only applied to the next future release.

Revision history for this message
Ben Shum (bshum) wrote :

Rebased branch here: user/bshum/lp1681943_rebase

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/bshum/lp1681943_rebase

I signed off on Galen's tweak, and then added a followup patch of my own too. There were hardcoded references to #ccc color in the file. Checking the stock colors.tt2 file, I can see that the variable should be [% css_colors.accent_light %]

Revision history for this message
Ben Shum (bshum) wrote :

Added another extra commit to alter the rest of the style.css.tt2 changes to include handling for RTL in master style.

Revision history for this message
Terran McCanna (tmccanna) wrote :

Thank you, Ben!

Revision history for this message
Galen Charlton (gmc) wrote :

Looking at this again and thinking about Dan's point about designs that hide data on mobile views... there's one field in particular that now gets hidden that is arguably a problem: the local call number. In particular, consider the use case of something using a list or temporary list on their cell phone to go out and retrieve books.

Fortunately, we have precedent in the form of the current checkouts view of a responsive table in the public catalog, so I'm going to try my hand at building on this and make the table responsive.

Noting this as an example of a way to do it that doesn't require copying the table headings into the stylesheet: https://codepen.io/AllThingsSmitty/pen/MyqmdM

Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
Revision history for this message
Galen Charlton (gmc) wrote :

I've pushed to user/gmcharlt/lp1681943_more_responsive_tables a branch that tweaks the display as promised in comment 12. Tossing back for one more pair of ideas before I merge this.

Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
Revision history for this message
Terran McCanna (tmccanna) wrote :

I have tested this code and consent to signing off on it with my name, Terran McCanna, and email address, <email address hidden>.

Thank you to both Galen and Ben!

Revision history for this message
Galen Charlton (gmc) wrote :

I've pushed this to master and rel_2_12. I don't propose to backport this to rel_2_12, so have added a WONTFIX, but I don't have an objection to another committer taking on the backport. Thanks, Terran and Ben!

Changed in evergreen:
importance: Wishlist → Medium
status: Confirmed → Fix Committed
Revision history for this message
Galen Charlton (gmc) wrote : Re: [Bug 1681943] Re: Improve Responsive Design in My Lists

> I've pushed this to master and rel_2_12. I don't propose to backport
> this to rel_2_12

Rather, I don't propose to backport this to rel_2_11.

Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.