Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.1] Template Override switches #36366

Merged
merged 5 commits into from
Jan 6, 2022
Merged

Conversation

brianteeman
Copy link
Contributor

For some reason the switches were not displayed in the same way as other similar switches. This leads to confusion (reported elsewhere) and the need for creating additional css.

This Pr resolves that. Made against 4.1 as there are language string removals and additions and I dont know what the policy is for that in j4.

Before

image

After

image

cc @Scrabble96

For some reason the switches were not displayed in the same way as other similar switches. This leads to confusion (reported elsewhere) and the need for creating additional css.

This Pr resolves that. Made against 4.1 as there are language string removals and additions and I dont know what the policy is for that in j4.

### Before

### After
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.1-dev labels Dec 21, 2021
@joomla-cms-bot joomla-cms-bot added the NPM Resource Changed This Pull Request can't be tested by Patchtester label Dec 21, 2021
@Quy
Copy link
Contributor

Quy commented Dec 21, 2021

PHP Notice: Undefined property: Joomla\Component\Templates\Administrator\View\Template\HtmlView::$mediaFiles in \administrator\components\com_templates\tmpl\template\default.php on line 100

Warning: count(): Parameter must be an array or an object that implements Countable in \administrator\components\com_templates\tmpl\template\default.php on line 100

@brianteeman
Copy link
Contributor Author

@Quy I cannot replicate your error report and there is nothing in this PR remotely related to the line mentioned

@Quy
Copy link
Contributor

Quy commented Dec 21, 2021

Sorry false alarm. Applied to v4.0. OK in v4.1.

@@ -42,6 +42,7 @@
label="COM_TEMPLATES_LAYOUTS_DIFFVIEW_CORE"
layout="joomla.form.field.radio.switcher"
default="0"
filter="boolean"
Copy link
Contributor

@sandewt sandewt Dec 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a boolean ? Is this an appointment within Joomla ?

See https://www.php.net/manual/en/language.types.boolean.php

@Quy
Copy link
Contributor

Quy commented Dec 22, 2021

I have tested this item ✅ successfully on b17288b


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

@sandewt
Copy link
Contributor

sandewt commented Dec 23, 2021

I want to test, but I can't find these switches. 😅
How to do?


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

@brianteeman
Copy link
Contributor Author

@sandewt They only display when you are working on a template override. So use the create override functionality and then select one of the newly created overrides and then you will see the switches in the top right

@sandewt
Copy link
Contributor

sandewt commented Dec 23, 2021

With a little trick I can make the buttons visible and get the following. The text is not inline with the buttons. This depends on the screen size.

afbeelding

@Quy
Copy link
Contributor

Quy commented Dec 30, 2021

@sandewt Please run npm i to compile the CSS changes or download/install the prebuilt package.

@chmst
Copy link
Contributor

chmst commented Jan 2, 2022

I am not in favour of this kind of switches. Switches should be for yes/no only (I know that we have several on other places, and i think it is always wrong).

Why not use a label? like "Hide original file: yes/no?"

@brianteeman
Copy link
Contributor Author

I know that we have several on other places, and i think it is always wrong).

so propose a change. this pr is consistent with current usage

@chmst
Copy link
Contributor

chmst commented Jan 2, 2022

What is current usage?

@brianteeman
Copy link
Contributor Author

What is current usage?

Verbs shoud be avoided in labels

@chmst
Copy link
Contributor

chmst commented Jan 2, 2022

Probably there is a reason for this rule, and maybe I am wrong with my my personal opinion, that switches should be used only for yes/no and lists for text-alternatives.

If you were German I could help very easily, we can make a noun from every verb :) "Versteckthaltung der Originaldatei"

I will start a discussion for that, because I think it is important. But not directly related to this PR.

@chmst
Copy link
Contributor

chmst commented Jan 2, 2022

I have tested this item ✅ successfully on 9b37b14


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

@chmst
Copy link
Contributor

chmst commented Jan 2, 2022

This solution is better than before.


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

@Quy
Copy link
Contributor

Quy commented Jan 3, 2022

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 3, 2022
@bembelimen bembelimen merged commit f035558 into joomla:4.1-dev Jan 6, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 6, 2022
@bembelimen
Copy link
Contributor

Thx

@bembelimen bembelimen added this to the Joomla 4.1 milestone Jan 6, 2022
@brianteeman
Copy link
Contributor Author

thanks

@brianteeman brianteeman deleted the diff branch January 6, 2022 12:06
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.

None yet

6 participants