Theme selector strings need updating and unset button required to unset theme

This issue was duplicated by another user, but one good suggestion from them was about the text on the first button mentioned. Instead of "Select device type", we could say "Show themes for this device type".

Hi,

I just ended with one small patch related with this @ MDL-29552 (it fixes another little detail, not listed here).

But when I was testing it, I discovered that it's impossible to "un-select" themes for device types and perhaps it should be something to consider. Just dropping the request here for your consideration.

Ciao

Bumping this as it has been duplicated again.

There are some additional details in duplicate MDL-30952 that might need to be considered.

Hi Michael, like I said in the duplicated issue you just closed, this is NOT a theme problem, although it is related to themes indirectly, this falls under Administration umbrella surely. Besides, this problem has become greater now in Moodle 2.2 as the option to select other device types, in a two-step fashion, you can't. It's not working as it did. I think Moodle 2.1 works OK still, but Moodle 2.2 seems to have regressed in this respect.

Cheers
Mary

Hi, Mary.

Sorry if you though I was dumping work on you. You are correct, this isn't about themes, it's more about the admin interface.

Thanks for assigning this to Raj. we'll handle this here at HQ.

Michael d;

Have added three commits:

1. Strings changed on bothe theme pages
2. Navigation added
3. Un-select theme option added for devices (except default device)

Just wondering, if we should have device name as lang string.

@Helen:
Can you please check if updated strings are fine. I have changed string on button to "Change theme" and not one described in bug.
IMO, Buttons should have small label.

Hi Raj,

Please could 'Select Device Type' be changed to 'Select device type' - i.e. only capitalizing the first word as per http://docs.moodle.org/dev/Coding_style#Language_strings

Also wondering whether it is necessary to have lang strings 'Select device type' AND 'Select device'? Maybe it is (I don't know) as I notice the strings are in different lang files.

Thanks for pointing that Helen.
You are right, we don't need two strings, hence one removed. Also, fixed capitalization string.

Looks good
+1 to integrate!
Thanks

Thanks Ankit,

I will put this up for integration review.

Uhm,

sorry but, or I'm reading/getting wrongly or I think we are creating some UI redundancies I'm not sure if they are a good idea.

1) You've added one extra admin category to enclose all the "theme selection" items, perfect, although it's one more level (prone to warp).

2) You've created one admin item for each device type, in order to replace the 2-screens setup, with a direct navigation to the picked device (1-less click). Oki.

3) But, while the "2-screens" approach has been replaced with the direct navigation alternative in 2) for setting and changing themes, at the same time you are keeping the old "2-screens" UI just for deleting one theme from the device.

IMO we should take rid of the "2-screens" completely and perform the deletion from each "device" page (but for "default", agree). To keep both UIs is really confusing, at least here.

So, I'd propose to:

1) Take rid of any link to the "2-screens" UI.
2) Add the "Delete" option (perhaps should be called "Unset", coz we aren't really deleting anything) to the Device theme picker (but to the "default" device).
3) Also, I've observed that, for the picked theme, it's shown inside a "black border" to mark it as the selected one. But, at the same time, the "Use theme" button continues being shown, but disabled or so. IMO we should not show it at all, it's already in use. Consider deleting it and/or changing it to one "Currently in use" message.
4) Also, the new admin pages are named "Change XXXX theme". IMO, both the "Change" and the "theme" are really unnecessary. Why not show the device type (surely with a complete "title") and done? That would prevent some warping in the block.
5) Finally, about code, apart of some commas not followed by whitespace, and some "//" comments not followed by whitespace (not sure if this is ruled), it looks correct.

So I'm reopening this to think a bit more about the way to avoid the "double UI" thingy together with the rest of minor points.

Ciao

Hi,
Can I make a suggestion...while these changes are on a backburner?

The need to change the wording on the Theme selector button came about because of confusion about the 2 tier page system in Theme selector. To the average user, who is used to just selecting a theme in the normal way, finds this changed page view very confusing.

Can we make the Theme setting option default NO for "Allow mobile device detection" thus making it the responsibility of the Admin to set this to YES if and ONLY IF needed. In my experience on the Themes forum this is one of things which confuses everyone, because it changes the way the Theme selector works.

If the Theme selector looks like it does normaly in earlier versions 1.9 and 2.0 then when folks update to 2.1.x or 2.2.x they can select their theme in the normal way without going into panic mode. If they then want to make their Moodle site detect Mobile devices they can do so in Theme settings?

It would also be great if once a theme has been set and the 'Continue' button clicked that the user is sent to the Home page, or at least given an option like on a course page "save and return" or "save and display", rather than just left hanging in the Theme selector.

I can see all of this needs some discussion.

Thanks
Mary

Thanks for the feedback Eloy and Mary,

1. I am not sure getting rid of 2-screen UI is back-portable on STABLE branches.
2. IMHO having two screen UI give user an overview of what theme has been selected for each device, and single point of control for viewing, deleting/unset theme.
3. Agree, we should not show disabled "Use theme" button
4. We can add unset theme on Device theme picker page. Probably, we should replace disabled "Use theme" button with unset button
5. Will change name of the pages with device type only.
6. "Allow mobile device detection" can be set to no for new installations and for upgrades from 1.9 and 2.0
7. I think it might be good to have two buttons to have, probably "Change theme" and "Continue", where "Change theme" will redirect user to changing the theme and "continue" should redirect user to home page.

I will try to have these changes in different commits, so that integrator's can take what they feel is fine.

After taking with Eloy, following will be fixed in this bug (and backported to stable barnches)

1. Replace string which are creating confusion (original issue)
2. Add "unset button"
3. Replace disable "use theme" with "Unset"

For master I will create another issue, which should take care of:

1. 2-screen to 1 screen
2. enhance navigation
3. "Allow mobile device detection" set to no (by default)
4. Add two buttons for redirection (after the theme is selected)

Hello Ankit,
Can you please review this again.
Following has been fixed:

1. Strings updated as described in bug
2. Unset theme button added
3. Disabled "use theme" button replaced with "unset theme" button (In case on default device no button is displayed)
4. "Clear theme cache" button will now redirect properly to the page it was pressed.

Hi Rajesh,
Looks good,
Am not sure but do we allow relative urls?

new moodle_url('/theme/index.php', array('device' => $device, 'unsettheme' => true, 'sesskey' => sesskey()));

or its better to use $CFG->root instead?
Thanks

@Ankit

We use new moodle_url a lot in themes rather than $CFG->wwwroot, especially in settings pages, if this helps? It seems that $CFG->wwwroot has been superseded by new moodle_url...but I may be wrong so don't quote me!

Cheers
Mary

Just testing this and think there is still room for improvement.

Suggestions:

I think it would look better if we make "Allow device type" in Theme settings default to NO this then would stop the confusion about where to set the default theme. We need to make this User friendly so how about 'Choose your default theme' as the header and (not 'Select default theme')

This then leaves us free to look at the 4 options where themes may need selecting.

However, since there is only ONE Mobile theme in CORE I think it would make sense to make that the default for Tablet and Mobile. And since 'Standard legacy' is the only Legacy theme available I feel this should be made the default for Legacy. This leave only the Default theme which is 'Standard'.

Now this would set the scene for the Administrator who would see at a glance what they need to change (if any), and as it is almost set in stone the only one they need to change would be the Default for their Main site. So perhaps here too we should change Device type default to "Main site theme"

Also can we put a capital letter in front of Default, Legacy, Tablet, Mobile if nothing other than to make them stand out a little more than they do on the Device selector page?

Thanks
Mary

What I said before, I forgot you have got this all in hand on different commits...so ignore the most part of it.

However...here is something I did today playing with CSS3, checkout the images, I have just uploaded, of the Device Theme selector.

Cool eh?

Wow, Looks awesome... Great work Mary

Thanks for the feedback Mary and Ankit.

@Ankit: moodle_url internally converts relative url to absolute url, So it's all good.

@Mary: I agree that core just have one mobile and legacy theme, but if we want to set default theme, then we should consider that for master only. So adding one more task to MDL-31927. Saying so, IMO tablet theme should not be same as mobile theme, so we should not set tablet theme. As per capitalization, sorry it doesn't go with moodle standards, check Helen's comments

Submitting for integration review again, with branches re-based.

Sorry Mary, Helen just pointed me about capitalization thing. You meant to have it on device selection page and I took it for theme selection page.
I will fix this and update the branches.

Thanks Helen and Mary for all the feedback

Yes, it all looks very cool Mary!

Regarding your comment "Also can we put a capital letter in front of Default, Legacy, Tablet, Mobile" I assume it's for on http://tracker.moodle.org/secure/attachment/27254/MDL-27862-DEVICETHEMES.jpg and if so, I agree with you that the words should be capitalized as they're first. (It's when they're in the middle of a phrase that they shouldn't be capitalized.)

Thanks Helen,

Added one more commit to make first letter capital and same font as theme name (under Information)

Some hours ago...

the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Branches re-based.

thanks for this change, it looks good to me and seems to test fine (not completely tested).

I've integrated this (functionality) into master, 22 and 21.

I've backported this functionality to stable branches based on the trivial addition of this functionality (which solves a bug).

An update to stable+dev docs will be needed.

Test result: Success.

It was good to see this through to the end.

Tested under Moodle 2.1, 2.2 and master.

FCT (fixed, closing, thanks). Ciao

"I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!"
~ Benjamin Disraeli