Skip to content

[4.0] Fix switch functionality for template overrides.#26359

Merged
wilsonge merged 6 commits intojoomla:4.0-devfrom
Harmageddon:fix_tpl_override_diff
Sep 24, 2019
Merged

[4.0] Fix switch functionality for template overrides.#26359
wilsonge merged 6 commits intojoomla:4.0-devfrom
Harmageddon:fix_tpl_override_diff

Conversation

@Harmageddon
Copy link
Contributor

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

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Sep 19, 2019
@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Sep 19, 2019
@Harmageddon Harmageddon changed the title Fixed switch functionality for template overrides. [4.0] Fix switch functionality for template overrides. Sep 19, 2019
@richard67
Copy link
Member

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.

@infograf768
Copy link
Member

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

Display looks weird to me.

@Harmageddon
Copy link
Contributor Author

@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?

@infograf768
Copy link
Member

@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

@brianteeman
Copy link
Contributor

There is already a PR for the double text

@Harmageddon
Copy link
Contributor Author

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.

@richard67
Copy link
Member

@Harmageddon Persian is RTL and should work.

@Harmageddon
Copy link
Contributor Author

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? ;-)

@infograf768
Copy link
Member

will do tomorrow

@infograf768
Copy link
Member

infograf768 commented Sep 21, 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==

@infograf768
Copy link
Member

See Tree correction in #26367

@Harmageddon
Copy link
Contributor Author

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

@infograf768
Copy link
Member

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.

@infograf768
Copy link
Member

@richard67 please test again

@richard67
Copy link
Member

@infograf768 Am already preparing my environment for this 👅

@richard67
Copy link
Member

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.

@richard67
Copy link
Member

@infograf768 RTC ;-)

@Quy
Copy link
Contributor

Quy commented Sep 21, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 21, 2019
@wilsonge wilsonge merged commit 58f50f7 into joomla:4.0-dev Sep 24, 2019
@wilsonge
Copy link
Contributor

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 24, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants