#1870944: [Meta] Mobile friendly admin pages

Problem/Motivation

Admin URL: admin/appearance/settings/bartik

  1. The Colour Picker doesnot work on smaller screens. A scroll is introduced close to 600px and when the width is further reduced the colour picker overlaps on the form.
  2. The Layout Preview needs to be responsive.

Proposed resolution

To be determined.

Remaining tasks

Tested in LTR browser, still needs testing in RTL.

Files: 
CommentFileSizeAuthor
#38 bartik-color-settings-1872598-38.patch3.95 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 58,842 pass(es).
[ View ]
#38 bartik-color-wide-2.png70.06 KBechoz
#38 bartik-color-320-2.png71.27 KBechoz
#39 bartik-color-320-b.png70.79 KBechoz
#37 bartik-colour-rtl-wide.png67.27 KBwill_richards
#37 bartik-colour-rtl-320.png63.91 KBwill_richards
#35 bartik-color-settings-1872598-35.patch3.81 KBechoz
PASSED: [[SimpleTest]]: [MySQL] 58,736 pass(es).
[ View ]
#35 bartik-color-wide.png69.02 KBechoz
#35 bartik-color-320.png72.09 KBechoz
#30 drupal-bartik-screen.png86.54 KBge
#27 color-rtl.png65.44 KBdjbobbydrake
#27 bartik-settings-page-on-small-screens-1872598-27.patch8.15 KBdjbobbydrake
PASSED: [[SimpleTest]]: [MySQL] 55,700 pass(es).
[ View ]
#26 drupal8-persian.png63.09 KBrteijeiro
#21 bartik-settings-page-on-small-screens-1872598-21.patch7.96 KBLewisNyman
PASSED: [[SimpleTest]]: [MySQL] 53,884 pass(es).
[ View ]
#15 bartik-settings-page-on-small-screens-1872598-15.patch6.99 KBLewisNyman
PASSED: [[SimpleTest]]: [MySQL] 53,292 pass(es).
[ View ]
#14 bartik-mobile-settings.png227.2 KBrteijeiro
#13 bartik-settings-page-on-small-screens-1872598-13.patch6.7 KBLewisNyman
PASSED: [[SimpleTest]]: [MySQL] 53,291 pass(es).
[ View ]
#12 bartik-settings-iphone.png261.08 KBrteijeiro
#12 bartik-settings.png129.92 KBrteijeiro
#5 bartik-settings-page-on-small-screens-1872598-5.patch655 bytesrachel_norfolk
PASSED: [[SimpleTest]]: [MySQL] 49,289 pass(es).
[ View ]
#4 bartik-settings-page-on-small-screens-1872598-4.patch397 bytesrachel_norfolk
PASSED: [[SimpleTest]]: [MySQL] 49,353 pass(es).
[ View ]

Comments

vijay.cgs’s picture

Category:task» bug

Proposed resolution

Change position absolute from position static using media queries for id #placeholder in core/modules/color/color.admin.css file

Old CSS

#placeholder {
  position: absolute;
  top: 0;
  right: 0; /* LTR */
}

New CSS

@media all and (max-width: 799px) {
  #placeholder {
    position:static;
  }
}
Shyamala’s picture

Issue summary:View changes

Added link to meta issue

LewisNyman’s picture

Issue tags:+mobile-novice, +css-novice

Adding novice tags

rachel_norfolk’s picture

Assigned:Unassigned» rachel_norfolk

I'll put a patch together for #1 and run it on iPhone etc (at Drupal Sprint in London...)

rachel_norfolk’s picture

StatusFileSize
new397 bytes
PASSED: [[SimpleTest]]: [MySQL] 49,353 pass(es).
[ View ]

Here's the patch so far...

rachel_norfolk’s picture

Status:Active» Needs review
StatusFileSize
new655 bytes
PASSED: [[SimpleTest]]: [MySQL] 49,289 pass(es).
[ View ]

Okay, so the first patch fixed the issue but I noticed that the following form elements were not resizing appropriately for mobile. I've improved them, at least on iPhone. Needs testing against other devices at least.

There is also some js that highlights the appropriate form element when adjusting the colour. It is not lining up with the correct form elements in mobile views and the js is a little beyond my (current) skills...

rachel_norfolk’s picture

Assigned:rachel_norfolk» Unassigned
crazyrohila’s picture

I think #1 and this should work.

/core/themes/bartik/color/preview.css.
/* ---------- Color form ----------- */
#color_scheme_form #palette .form-item {
+ max-width: 100%;
width: 25em;
}
#color_scheme_form #palette .form-item label {
+ max-width: 100%;
width: 15em;
}

LewisNyman’s picture

Ok, Ive had a look at the surrounding CSS in color.admin.css and bartik/color/preview.css

I don't think we need to touch the Javascript, the main issue is that the default, non media query css is really not set up to be fluid, a lot of fixed em based widths and fixed heights that don't play well with fluid layouts. The problem with the highlighting of form elements breaks because there is a height set on line 18 of color.admin.css. Your media query CSS looks like it's being overridden by ID selectors in bartik/color/preview.css

LewisNyman’s picture

Status:Needs review» Needs work
crazyrohila’s picture

I tried media query in preview.css.

+@media all and (max-width: 799px) {
+  #placeholder {
+    position: static;
+  }
+}

but Its overridden by color.admin.css in color module.
So should I put this in color.admin.css ?
(I am little afraid to edit core things)
And for fluid layout I set form-item max-width to 100% in preview.css in bartik.. this is working fine with small screen.
/* ---------- Color form ----------- */

#color_scheme_form #palette .form-item {
+  max-width: 100%;
   width: 25em;
}
crazyrohila’s picture

Status:Needs work» Needs review
rteijeiro’s picture

Status:Needs review» Needs work
StatusFileSize
new129.92 KB
new261.08 KB

Hi, tested #5 patch and it seems ok in Chrome, Firefox, Opera and Safari but it doesn't look well in iPhone. There is an element with wrong width as shown in the following image:

I have also noticed that some preview elements should be fixed in order to fix with small screen sizes, as shown in the following image:

LewisNyman’s picture

Status:Needs work» Needs review
StatusFileSize
new6.7 KB
PASSED: [[SimpleTest]]: [MySQL] 53,291 pass(es).
[ View ]

Ok, so I kind of hacked away at the color.admin.css with a machete. It should work pretty well on all screen sizes. I also ported over some of the responsive work done on Bartik into the preview.css.

I feel dirty, I'm going to take a shower.

rteijeiro’s picture

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new227.2 KB

Hey, great work, Lewis ;)

Reviewed in Chrome, Safari, Firefox and Opera. Also tested with an iPhone ;)

Now it seems ok, as shown in the screenshoot (sorry for the scrollbars):

Bartik settings

The only thing I noticed is that the color picker is not centered but I guess it's out of the scope of this issue ;)

Well done!!

Maybe RTBC?

LewisNyman’s picture

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new6.99 KB
PASSED: [[SimpleTest]]: [MySQL] 53,292 pass(es).
[ View ]

I've centered the colour wheel.

I would be morre concerned about the usability of the colour wheel on touch devices. It's just about usable but it's not great.. maybe we should open up another issue for that?

Bojhan’s picture

Heh, yes - lets split that off.

rteijeiro’s picture

Status:Needs review» Reviewed & tested by the community

Well done Lewis, now it looks perfect!!

It guess it's ready to be merged :)

webchick’s picture

Status:Reviewed & tested by the community» Needs work

This looks great!!

It looks like there are changes to the main stylesheets that affect(ed) RTL styling, but no corresponding hunks in a -rtl.css stylesheet. Methinks that's wrong. We should make sure to test this explicitly in an RTL language (e.g. Arabic or Hebrew).

webchick’s picture

Oh. And can we please get a follow-up issue (or cross-link it if one already exists) for making Color module work better on touch devices?

xjm’s picture

LewisNyman’s picture

Status:Needs work» Needs review
StatusFileSize
new7.96 KB
PASSED: [[SimpleTest]]: [MySQL] 53,884 pass(es).
[ View ]

Touch follow up: #1957400: Improve Color's admin interface on touch devices

Ah I forgot about RTL... I haven't figured out interdiffs but just check out the changes in color.admin-rtl.css. I've actually removed quite a bit of CSS because I re-styled some of the elements using display: inline-block so they should obey text-align changes.

This will need a proper test as I can't seem to install other languages on my d8 local

xjm’s picture

Issue tags:-Needs followup

Thanks @LewisNyman!

Shyamala’s picture

Issue tags:+Needs followup

Needs follow-up issue (or cross-link it if one already exists) for making Color module work better on touch devices? as highlighted by webchick

echoz’s picture

Issue tags:-Needs followup

follow-up was created in @21

Shyamala’s picture

Thanks @echoz, missed that :)

rteijeiro’s picture

Status:Needs review» Needs work
StatusFileSize
new63.09 KB

Tested RTL styles (with Persian language) and it seems need work in the color picker and color textboxes. The rest of the page is ok (see screenshoot)

djbobbydrake’s picture

StatusFileSize
new8.15 KB
PASSED: [[SimpleTest]]: [MySQL] 55,700 pass(es).
[ View ]
new65.44 KB

@LewisNyman - I amended your patch to address the RTL issues. Screenshot of resolved RTL issue is attached. You can see there's also an additional bug with the color codes. The "#", I'm assuming, should still be on the left. Will open up a separate issue.

color-rtl.png

djbobbydrake’s picture

Status:Needs work» Needs review

Setting this to Needs review. Also, issue with hex codes created here: http://drupal.org/node/2003570

drupal930’s picture

I received this message:
Fatal error: Call to a member function get() on a non-object in /home/scf08b589026e5c3/www/core/lib/Drupal.php on line 147

drupal930’s picture

Issue summary:View changes

edited meta link

ge’s picture

StatusFileSize
new86.54 KB

Fatal error is just with simplytest.me because the test bot is having problems.

Manually applied the patch and the display is fixed in iOS Simulator LTR. Testing in Chrome on Mac the display is fixed with the patch if the Overlay is turned off, but locks next to hex color areas switch from right side to left side when I resize the Chrome window as shown in the attachment.

Will still need to be tested in RTL browsers.

LewisNyman’s picture

I think we can veto the narrow-chrome-with-overlay bug because the overlay shouldn't be active at that size. If you reload the page and click on it after you resize the window the overlay shouldn't open. If someone thinks it's a use case then we should be checking the overlay width on the click even as well as on load.

Status:Needs review» Needs work
Issue tags:-Usability, -mobile, -Needs manual testing, -Responsive Design, -d8mux, -mobile-novice, -css-novice, -d8mux-admin

The last submitted patch, bartik-settings-page-on-small-screens-1872598-27.patch, failed testing.

djbobbydrake’s picture

echoz’s picture

Status:Needs review» Needs work
echoz’s picture

Status:Needs work» Needs review
Issue tags:-mobile-novice, -css-novice
StatusFileSize
new72.09 KB
new69.02 KB
new3.81 KB
PASSED: [[SimpleTest]]: [MySQL] 58,736 pass(es).
[ View ]

Re-wrote from scratch, mobile first FTW :-) Screenshots demo a selected item and the lock / hook thing.

wider viewport

bartik-color-wide.png

320px viewport

bartik-color-320.png

Someone please test RTL.

echoz’s picture

Title:Bartik Settings page in Small screens» Bartik color settings not responsive
Component:CSS» color.module
Issue tags:+Bartik
will_richards’s picture

StatusFileSize
new63.91 KB
new67.27 KB

Tested RTL wide and 320px viewport with selected item - both look great, but are missing hook image (css is there but image can't be found).

Wide Viewport

bartik-colour-rtl-wide.png

320px viewport

 bartik-colour-rtl-320.png

echoz’s picture

Issue tags:+Usability, +mobile, +Needs manual testing, +Responsive Design
StatusFileSize
new71.27 KB
new70.06 KB
new3.95 KB
PASSED: [[SimpleTest]]: [MySQL] 58,842 pass(es).
[ View ]

Thanks for the RTL testing! New patch.

RTL Wider viewport

bartik-color-wide-2.png

RTL 320px viewport

bartik-color-320-b.png

echoz’s picture

Issue tags:-Usability, -mobile, -Needs manual testing, -Responsive Design
StatusFileSize
new70.79 KB

Rather this is actually the 320 screenshot, editing above to display this one.

BarisW’s picture

Status:Needs review» Reviewed & tested by the community
Issue tags:-Usability, -mobile, -Needs manual testing, -Responsive Design

It looks great on the iPhone. I've tested English and Arabic (RTL), both in portrait and landscape.

Let's get this in.

echoz’s picture

Issue tags:+mobile

Lost an important tag

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Thanks for the screenshots!

Committed 54cc3d7 and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary:View changes

Updated summary.