? Failure

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
12 Jul 2017

Currently we have a column for the language on all list views and a field to select the language on all edit views

This PR removes them when you are on a non multilingual site. This simplifies the UI and removes a useless field when creating content

ToDo

  • Lists
  • Forms
  • Filters waiting for #17355 to be merged
  • Search tools

After PR and non multilingual

screenshotr10-20-00

screenshotr10-20-55

Before PR and After PR and multilingual

screenshotr10-22-36

screenshotr10-22-46

d4d25d7 12 Jul 2017 avatar brianteeman cells
7e7b96d 12 Jul 2017 avatar brianteeman th
avatar brianteeman brianteeman - open - 12 Jul 2017
avatar brianteeman brianteeman - change - 12 Jul 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 12 Jul 2017
Category Administration com_associations com_banners com_categories com_contact com_content com_fields com_menus com_modules com_newsfeeds com_tags Layout
avatar brianteeman brianteeman - change - 12 Jul 2017
The description was changed
avatar brianteeman brianteeman - edited - 12 Jul 2017
avatar infograf768
infograf768 - comment - 12 Jul 2017

Once more: some 3pd may use other language systems to set a multilingual site.
In that case, they may not use the languagefilter plugin.
Therefore using if (JLanguageMultilang::isEnabled()) as a conditional would prevent users working with their extensions to see the language column or assihn content languages to items.

avatar infograf768
infograf768 - comment - 12 Jul 2017

A possible solution to this is to create a new Global Configuration parameter.
Multilingual: Yes/no

Then we can

  1. OR force JLanguageMultilang::isEnabled() to include this parameter and create messages to inform users that enabling the languagefilter plugin is not enough to get the site multilingual, or manage to set that Global Parameter automatically when the plugin is enabled.

  2. OR create only this new parameter and then in your conditional, use
    `if (JLanguageMultilang::isEnabled() || JFactory::getConfig()->get('multilingual') == 1)

avatar brianteeman
brianteeman - comment - 12 Jul 2017

This is j4. If they do it a non standard way then they will have to change. Joomla has an api and we must use it. We don't need any new parameters we have too many options as it is.

avatar infograf768
infograf768 - comment - 12 Jul 2017

I guess 4.0 lead maintainers will take the decision. IMHO, Joomla should remain open.

avatar mbabker
mbabker - comment - 12 Jul 2017

We should have one API method to determine if multilingual functionality is available. Whether it's the core code or a third party extension, that code needs to be able to tell that API method the functionality is enabled. If that functionality is disabled, we should rightfully be able to remove things from the UI that would not be required without it.

So this is fine for 4.0. We aren't closing Joomla by any means. What this probably results in as things progress is changing our existing methods to make them more capable of working with third party solutions versus their current hardcoded manner of relying on one core plugin.

avatar brianteeman
brianteeman - comment - 12 Jul 2017

Thank you for your wisdom @mbabker finally we can make progress on using joomla

avatar infograf768
infograf768 - comment - 12 Jul 2017

What this probably results in as things progress is changing our existing methods to make them more capable of working with third party solutions versus their current hardcoded manner of relying on one core plugin.

Good. Then I guess that JLanguageMultilang::isEnabled() will have to change.

avatar mbabker
mbabker - comment - 12 Jul 2017

Yep. As a temporary thing (until we come up with a more eloquent solution), this would be the change I make:

diff --git a/libraries/src/CMS/Language/Multilanguage.php b/libraries/src/CMS/Language/Multilanguage.php
index d6d92b7..066280f 100644
--- a/libraries/src/CMS/Language/Multilanguage.php
+++ b/libraries/src/CMS/Language/Multilanguage.php
@@ -18,6 +18,14 @@ defined('JPATH_PLATFORM') or die;
 class Multilanguage
 {
 	/**
+	 * Flag indicating multilanguage functionality is enabled.
+	 *
+	 * @var    boolean
+	 * @since  __DEPLOY_VERSION__
+	 */
+	public static $enabled = false;
+
+	/**
 	 * Method to determine if the language filter plugin is enabled.
 	 * This works for both site and administrator.
 	 *
@@ -30,8 +38,11 @@ class Multilanguage
 		// Flag to avoid doing multiple database queries.
 		static $tested = false;
 
-		// Status of language filter plugin.
-		static $enabled = false;
+		// Do not proceed with testing if the flag is true
+		if (static::$enabled)
+		{
+			return true;
+		}
 
 		// Get application object.
 		$app = \JFactory::getApplication();
@@ -39,9 +50,9 @@ class Multilanguage
 		// If being called from the frontend, we can avoid the database query.
 		if ($app->isClient('site'))
 		{
-			$enabled = $app->getLanguageFilter();
+			static::$enabled = $app->getLanguageFilter();
 
-			return $enabled;
+			return static::$enabled;
 		}
 
 		// If already tested, don't test again.
@@ -57,11 +68,11 @@ class Multilanguage
 				->where($db->quoteName('element') . ' = ' . $db->quote('languagefilter'));
 			$db->setQuery($query);
 
-			$enabled = $db->loadResult();
+			static::$enabled = $db->loadResult();
 			$tested = true;
 		}
 
-		return (bool) $enabled;
+		return (bool) static::$enabled;
 	}
 
 	/**
@@ -110,3 +121,4 @@ class Multilanguage
 		return $multilangSiteHomePages;
 	}
 }
+

Then anyone can call JLanguageMultilang::$enabled = true; to turn on the system. If someone doesn't turn it on, then it falls back to the existing checks (which admittedly favors the core API and plugin).

avatar brianteeman
brianteeman - comment - 12 Jul 2017

@wilsonge in order to complete the removal of the language sort and search filters we need something like #8767 so that we can conditionally remove them - can you help me out on that?

avatar brianteeman
brianteeman - comment - 12 Jul 2017

Once more: some 3pd may use other language systems to set a multilingual site.

Can you please point to one of these systems as everything i tested on the jed uses the plugin

avatar brianteeman brianteeman - change - 14 Jul 2017
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 14 Jul 2017
Category Administration com_associations com_banners com_categories com_contact com_content com_fields com_menus com_modules com_newsfeeds com_tags Layout Administration com_associations com_banners com_categories com_contact com_content com_cpanel com_fields com_menus com_modules com_newsfeeds com_tags Language & Strings Modules Templates (admin)
avatar matrikular
matrikular - comment - 17 Jul 2017

Just a simple note: If we're going to do this, we would also have to make the colspan count for every list footer variable / dynamic.

Is there still time to split this into PRs on a "per extension basis" or at least to reduce the amount of files to review / test somehow? Personally, after the NNth file, I loose track.

What is the relation of the following code to this PR:
https://github.com/joomla/joomla-cms/pull/17086/files#diff-c59be36b2499577a7e6339384f5695c5R17

avatar brianteeman
brianteeman - comment - 17 Jul 2017

Just a simple note: If we're going to do this, we would also have to make the colspan count for every list footer variable / dynamic.

The footer? Can you show me the code that is effected by this

What is the relation of the following code to this PR:

Thats an error in the PR from trying to solve conflicts and will be resolved if this is accepted

avatar matrikular
matrikular - comment - 17 Jul 2017

Sure.

This affects every list view like com_banners e.g.:
https://github.com/brianteeman/joomla-cms/blob/3723eed456ca12931411050f5a7ecc30f0a0f3ce/administrator/components/com_banners/tmpl/banners/default.php#L84

Looking at it, it seems that we have already an error in the colspan count. While there are 10 columns, the footer colspan is set to 13. With your PR, it should be 9 or 10 (depending on the language check).

The same goes for com_contact: https://github.com/brianteeman/joomla-cms/blob/3723eed456ca12931411050f5a7ecc30f0a0f3ce/administrator/components/com_contact/tmpl/contacts/default.php#L81

Or com_categories: https://github.com/brianteeman/joomla-cms/blob/3723eed456ca12931411050f5a7ecc30f0a0f3ce/administrator/components/com_categories/tmpl/categories/default.php#L136

Note that com_categories already uses a variable that is influenced by the icon columns.

avatar brianteeman
brianteeman - comment - 17 Jul 2017

@matrikular thanks - I see what you mean now but as you said this error already exists before thiis PR and imho should be fixed in another PR

avatar matrikular
matrikular - comment - 17 Jul 2017

If you change the column count, you will have to change the colspan count as well. The fact that com_banners shows the wrong number has nothing to do with it.

avatar brianteeman
brianteeman - comment - 19 Jul 2017

@matrikular from my research its not needed to reduce the colspan on the footer. As long as the colspan is equal to or greater than the number of columns it will span the entire width.

avatar brianteeman brianteeman - change - 19 Jul 2017
Milestone Added:
avatar brianteeman brianteeman - change - 31 Jul 2017
The description was changed
avatar brianteeman brianteeman - edited - 31 Jul 2017
avatar joomla-cms-bot joomla-cms-bot - change - 31 Jul 2017
Category Administration com_associations com_banners com_categories com_contact com_content com_fields com_menus com_modules com_newsfeeds com_tags com_cpanel Language & Strings Modules Templates (admin) Administration com_associations com_banners com_categories com_contact com_content com_cpanel com_fields com_installer com_menus com_modules com_newsfeeds
avatar brianteeman
brianteeman - comment - 31 Jul 2017

closed temperarily

avatar brianteeman brianteeman - close - 31 Jul 2017
avatar brianteeman brianteeman - change - 31 Jul 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-07-31 15:52:03
Closed_By brianteeman
Labels Removed: ?
avatar brianteeman brianteeman - change - 24 Aug 2017
Status Closed New
Closed_Date 2017-07-31 15:52:03
Closed_By brianteeman
avatar brianteeman brianteeman - change - 24 Aug 2017
Status New Pending
avatar brianteeman brianteeman - reopen - 24 Aug 2017
avatar brianteeman
brianteeman - comment - 24 Aug 2017

re-opened to resolve conflicts

avatar brianteeman
brianteeman - comment - 29 Aug 2017

closed as the core code has changed too much for an easy merge/update - will redo it at some point

avatar brianteeman brianteeman - change - 29 Aug 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-08-29 17:17:29
Closed_By brianteeman
avatar brianteeman brianteeman - close - 29 Aug 2017
avatar brianteeman
brianteeman - comment - 30 Aug 2017

See #17765

Add a Comment

Login with GitHub to post a comment