Beautify install screen on mobile phone

PATCH!

+++ b/core/themes/seven/js/mobile-install-steps.jsundefined
@@ -0,0 +1,24 @@
+function installStepsSetup() {
+  var steps = document.querySelectorAll('.task-list li');
+  var branding = document.querySelector('#branding');
+  var numberOfSteps = steps.length;
+  var activeStep = findActiveStep(steps);
+  var stepIndicator = document.createElement('div');
+  stepIndicator.className = 'step-indicator';
+  stepIndicator.innerText = activeStep + '/' + numberOfSteps;
+  branding.appendChild(stepIndicator);
+}
+
+function findActiveStep(steps) {
+  var activeStep = false;
+  for (var i = 0; i < steps.length; i++) {
+    if(steps[i].className == 'active') {
+      activeStep = i + 1;
+    }
+  }
+  return activeStep;
+}
+
+document.addEventListener('DOMContentLoaded', function() {
+   installStepsSetup();

This entire block would be so much more simple if it were using jQuery. Why use plain Javascript? If we're not going to use jQuery, then there needs to be some comments, as it's quite dense. However, I'm pretty sure that IE8 does not support DOM level 2, which is necessary for document.addEventListener, so maybe look at using jQuery instead of the addEventListener call?

+++ b/core/themes/seven/js/mobile-install-steps.js
@@ -0,0 +1,24 @@
\ No newline at end of file

Needs newline :)

+++ b/core/themes/seven/template.php
@@ -116,3 +116,12 @@ function seven_css_alter(&$css) {
+
+/**
+ * Overrides theme_install_page().
+ */
+function seven_install_page($variables) {
+  drupal_add_js(drupal_get_path('theme', 'seven') . '/js/mobile-install-steps.js', 'file');
+  drupal_add_http_header('Content-Type', 'text/html; charset=utf-8');
+    return theme('maintenance_page', $variables);
+}
\ No newline at end of file

Is there a reason to manually add that http header here? Also, needs newline.

It needs a closure. and "use strict".

I asked for not using jQuery here. It's not needed, this is targeted at mobile and they understands DOM2.

this is targeted at mobile and they understands DOM2.

That's true, but this Javascript is loaded unconditionally on all browsers and it'll throw a Javascript error on browsers that do not support it.

At the mobile sprint we figured that if Drupal is completely mobile friendly you should be able to install it on your mobile phone.

Given that more and more users (especially world-wide) only have a mobile device and not a larger-screened computer, this makes a hell of a lot of sense. Thanks for taking this on!

Hmm... wouldn't a finger swipe be the same as "Save and continue" (which to me is the same as "Next"). You would also want to be able to see the previous option.

I agree this is scope creep. I have concerns about whether this idea would even be a good idea. But if you'd like to discuss, let's move it to a separate follow-up issue.

nod_'s request to not use jQuery here is inline with the JavaScript decisions we've been discussing for several weeks (months?). We cannot make a change to this approach to JS at this late date before code freeze. If you'd like to re-argue that, can we at least push the argument off to past December 1? :-)

Also, IE8 should be getting the full-on desktop installation experience that doesn't use this new JS.

Is there a reason to manually add that http header here?

Yes. We're overriding theme_install_page(), so we need to keep all the original code from that theme function. :-)
See http://api.drupal.org/api/drupal/core%21includes%21theme.maintenance.inc...

Updated patch.

1. The "Finished" step was showing a status of "false / 7" steps completed. :-) This is because the final step is never "active", it's already "done" when you land on it. I added some logic to fix that.
2. Removed 2 variables from the JS since they were created and only used once. Makes the code easier to follow this way, too.
3. Confirmed that the JS causes a "do you want to debug this JS" message on IE8 (depending on settings), so it's definitely throwing an error there. I wrapped the offending JS in an if statement to check if the DOM method exists before calling it.
4. Fixed the "No newline at end of file" problems with the previous patch.

Adding tag, so it shows up in "mobile admin" list on http://groups.drupal.org/mobile/drupal-8

Tested on Chrome. Attached screenshots.

Tested on Chrome. Attached screenshots.

Having this tested on multiple devices should help, before moving to RTBC. I can have it tested on Monday on a couple of devices.

Quick reroll

renamed the file to mobile.install.js we don't use hyphens anywhere else. Added the script in seven_preprocess_install_page() instead of taking over the theme. Replaced .innerText with .innerHTML since it doesn't work in Firefox. Made the findStep() function return as soon as it finds a result to avoid looping over everything.

#16: core-js-install-steps-1832568-17.patch queued for re-testing.

Looks like a real test failure. Anyone available to finish off this patch?

Looked into this, and do not see how this can be failing. Nothing to do with this patch. Have tested on local machine clean install with patch applied, run the test that failed and it passes.

Maybe try one more shot at the test bot.

#16: core-js-install-steps-1832568-17.patch queued for re-testing.

test ok now somehow.

Looks good, nice work

#16: core-js-install-steps-1832568-17.patch queued for re-testing.

+++ b/core/themes/seven/template.phpundefined
@@ -121,3 +121,10 @@ function seven_css_alter(&$css) {
+
+/**
+ * Implements hook_preprocess_install_page().
+ */
+function seven_preprocess_install_page(&$variables) {
+  drupal_add_js(drupal_get_path('theme', 'seven') . '/js/mobile.install.js');

Is this the only place it can go?

It's very much tied to the markup of the install screen since we're getting the number of steps and current step from the markup and not some JS settings.

I'd assume that a distribution would override that since the markup could/will be different.

So yes to me that's only on seven theme install pages that it should go. Maybe I missed a part of the question?

Previously it was taking over the theme function which was a bit more messy. It's not in a hook_library_info since there is no dependency for the script.

Sounds like catch's concern is addressed. This is a nice improvement.

Committed and pushed to 8.x. Thanks!