? NPM Resource Changed ? ? Pending

User tests: Successful: Unsuccessful:

avatar Harmageddon
Harmageddon
19 Sep 2019

Pull Request for Issue #25323.

Summary of Changes

This is a proposal to fix the functionality of the switchers in the template override view that had been broken with #24463.
@dgrammatiko I removed parts of your JS code including the observers, because of the different switch functionality now. Could you please have a look whether this might break anything?

The new admin template slightly changed the layout of this page, so the behaviour when all panels are open is not exactly the same as before.

Before

| Override | Original |
|       Diff          |

After

| Override | Original |
|   Diff   |

Testing Instructions

  1. Navigate to "System - Site Templates - Cassiopeia Details and Files".
  2. Create an override for a component view.
  3. Edit the override and insert or delete something.
  4. Toggle the "Show/Hide Diff" and "Show/Hide Original" switchers.
  5. With one or both switchers enabled, reload the page.

Expected result

An additional area containing the original, respectively the difference between override and original, or both should appear.
The state of every switcher should be saved after reloading the page.

Actual result

Nothing happens.

Documentation Changes Required

None

avatar Harmageddon Harmageddon - open - 19 Sep 2019
avatar Harmageddon Harmageddon - change - 19 Sep 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Sep 2019
Category Administration com_templates JavaScript Repository NPM Change
avatar Harmageddon Harmageddon - change - 19 Sep 2019
Labels Added: NPM Resource Changed ?
avatar joomla-cms-bot joomla-cms-bot - change - 19 Sep 2019
Category Administration com_templates JavaScript Repository NPM Change Administration com_templates Language & Strings JavaScript Repository NPM Change
avatar Harmageddon Harmageddon - change - 19 Sep 2019
Title
Fixed switch functionality for template overrides.
[4.0] Fix switch functionality for template overrides.
avatar Harmageddon Harmageddon - edited - 19 Sep 2019
avatar richard67 richard67 - test_item - 19 Sep 2019 - Tested successfully
avatar richard67
richard67 - comment - 19 Sep 2019

I have tested this item successfully on d118e72


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26359.

avatar infograf768 infograf768 - change - 20 Sep 2019
Labels Added: ?
avatar infograf768
infograf768 - comment - 20 Sep 2019

This is what I get
Screen Shot 2019-09-20 at 08 23 51

Display looks weird to me.

avatar Harmageddon
Harmageddon - comment - 20 Sep 2019

@infograf768 Are you referring to the fact that the switchers are only grey when deactivated? I fixed this in #26360. Or is there something else?

avatar infograf768
infograf768 - comment - 20 Sep 2019

@Harmageddon
Not only that but the fact that the labels Show Original file and Show Differences are redundant with the switcher state and are far away from the switcher. Also when window is narrowed, they go over the Editing File...etc.
The labels should be: Original File and Differences (and next to the Switcher) and the switcher could just use Show and Hide imho.

We have a similar issue in the Template Style Edit with label and switcher state where we should have as label Layout (once only and not twice...) instead of Fluid Layout, but in this case, happily, the label is over the switcher.
Screen Shot 2019-09-20 at 10 05 04

avatar brianteeman
brianteeman - comment - 20 Sep 2019

There is already a PR for the double text

avatar Harmageddon
Harmageddon - comment - 20 Sep 2019

Tbh, I'm not sure whether these changes belong here or in #26360, but I just included them here. Should look better now. Now, it is required to test the switchers in other places, too, to make sure I didn't mess them up. From global configuration and some modules, it looks fine to me.
Though I'm not sure about the RTL stuff, but as far as I see, there is no RTL language able to be installed, so I can't really check that.

avatar richard67
richard67 - comment - 20 Sep 2019

@Harmageddon Persian is RTL and should work.

avatar Harmageddon
Harmageddon - comment - 20 Sep 2019

Thanks @richard67! To me, it looks good now. But @infograf768, maybe you might want to check, as you are an expert of RTL compatibility IIRC? ;-)

avatar infograf768
infograf768 - comment - 20 Sep 2019

will do tomorrow

avatar infograf768
infograf768 - comment - 21 Sep 2019

Works quite fine. Thanks.
Just needs a small change in the switcher.scss

For a reason that I ignore, we have in LTR a padding-right: 5px; for .switcher__legend

if we don't want to keep this padding, we just have to delete it from the LTR part.
If we want to keep this, then we have to modify for RTL from:

.switcher__legend {
  margin-bottom: 1rem;
  font-size: 1rem;
  font-weight: 400;
  float: left;
  width: 220px;
  padding-top: 5px;
  padding-right: 5px;
  text-align: left;

  [dir=rtl] & {
    float: right; 
    text-align: right;
  }
}

to

.switcher__legend {
  margin-bottom: 1rem;
  font-size: 1rem;
  font-weight: 400;
  float: left;
  width: 220px;
  padding-top: 5px;
  padding-right: 5px;
  text-align: left;

  [dir=rtl] & {
    float: right;
    padding-left: 5px;
    padding-right: 0px; 
    text-align: right;
  }
}

NOTE

Just to remember. Unrelated to this patch.
In RTL we need to force use text-align: left; for the editor textarea.
We also need to correct the tree in administrator/index.php?option=com_templates&view=template&id=202&file=aG9tZQ==

avatar infograf768
infograf768 - comment - 21 Sep 2019

See Tree correction in #26367

avatar Harmageddon
Harmageddon - comment - 21 Sep 2019

You're absolutely right, thank you for the good spot @infograf768!

avatar infograf768 infograf768 - test_item - 21 Sep 2019 - Tested successfully
avatar infograf768
infograf768 - comment - 21 Sep 2019

I have tested this item successfully on 09be866


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26359.

avatar infograf768
infograf768 - comment - 21 Sep 2019

@richard67 please test again

avatar richard67
richard67 - comment - 21 Sep 2019

@infograf768 Am already preparing my environment for this ?

avatar richard67 richard67 - test_item - 21 Sep 2019 - Tested successfully
avatar richard67
richard67 - comment - 21 Sep 2019

I have tested this item successfully on 09be866


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26359.

avatar richard67
richard67 - comment - 21 Sep 2019

@infograf768 RTC ;-)

avatar Quy Quy - change - 21 Sep 2019
Status Pending Ready to Commit
avatar Quy
Quy - comment - 21 Sep 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26359.

avatar wilsonge wilsonge - change - 24 Sep 2019
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-09-24 10:11:39
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 24 Sep 2019
avatar wilsonge wilsonge - merge - 24 Sep 2019
avatar wilsonge
wilsonge - comment - 24 Sep 2019

Thanks!

Add a Comment

Login with GitHub to post a comment