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.0] Update location path in description #34238

Merged
merged 1 commit into from
May 29, 2021
Merged

Conversation

Quy
Copy link
Contributor

@Quy Quy commented May 26, 2021

Summary of Changes

The old path is for J3. Update location path for J4.

Testing Instructions

Code review.

or

Install Testing Sample Data.
Go to System > Manage > Extensions
Search and disable Wrapper - Site.
Go to System > Manage > Site Modules
Go to the Wrapper row.
Hover mouse over the disabled icon under Status column.

Actual result BEFORE applying this Pull Request

manage-extensions

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels May 26, 2021
@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on c1c2112

Marking as a successful test

but it really needs to be changed from a title to make it accessible and usable. Also you don't normally get information on anything disabled and the grey colour makes this look disabled so the title is unexpected


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

@richard67
Copy link
Member

I have tested this item ✅ successfully on c1c2112


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 26, 2021
@richard67 richard67 added this to the Joomla 4.0 milestone May 26, 2021
@infograf768
Copy link
Member

infograf768 commented May 27, 2021

Thought:

There are other ways to get to Manage -> Extensions than passing by the default admin Joomla menu System > Manage > Extensions .

For example, when using the Alternative Main Menu, it is Extensions > Manage > Manage
Or in case of a custom admin menu, it can be Manage > Extensions

All have evidently the same url index.php?option=com_installer&view=manage

It would be nice to use a link with the url as second variable in the sprintf.

This evidently if we can click on that link.

The problem is that it is a tip created as Title and therefore can't be clicked.
Any idea how to improve this?

@brianteeman
Copy link
Contributor

@infograf768 see #34240 that would allow your suggestion

@infograf768
Copy link
Member

I was thinking of something like this

extensions-manage

@infograf768
Copy link
Member

infograf768 commented May 27, 2021

code would be something like

diff --git a/administrator/components/com_modules/tmpl/modules/default.php b/administrator/components/com_modules/tmpl/modules/default.php
index 05a53b5..ebb7513 100644
--- a/administrator/components/com_modules/tmpl/modules/default.php
+++ b/administrator/components/com_modules/tmpl/modules/default.php
@@ -124,7 +124,15 @@
 							<?php else : ?>
 								<?php // Extension is not enabled, show a message that indicates this. ?>
-								<span class="tbody-icon" title="<?php echo Text::sprintf('COM_MODULES_MSG_MANAGE_EXTENSION_DISABLED', $this->escape($item->name)); ?>">
-									<span class="icon-minus-circle" aria-hidden="true"></span>
-								</span>
+								<div class="dropdown">
+									<button role="button" data-bs-toggle="dropdown" aria-expanded="false" class="icon-minus-circle btn-sm dropdown-toggle">
+									</button>
+									<ul class="dropdown-menu dropdown-menu-end" style="position: absolute; inset: 0px auto auto 0px; margin: 0px; transform: translate(-228px, 27px);" data-popper-placement="bottom-start">
+										<li>
+											<button type="button" class="dropdown-item" title="Extensions: Manage">
+												<a href="<?php echo Route::_('index.php?option=com_installer&view=manage'); ?>"><?php echo Text::sprintf('COM_MODULES_MSG_MANAGE_EXTENSION_DISABLED', $this->escape($item->name)); ?></a>
+											</button>
+										</li>
+									</ul>
+								</div>
 							<?php endif; ?>
 						</td>

@Quy
Copy link
Contributor Author

Quy commented May 28, 2021

While functional, it feels clunky. Maybe keep the tooltip and click the button to go to the Extensions list.

@infograf768
Copy link
Member

infograf768 commented May 28, 2021

Here is another solution.
The tooltip is now in a similar format as the other status icons tooltips (aria-labelledby).
Clicking on the icon redirects to Manage => Extensions
The only visual difference is the cursor. Therefore we may need to modify the tip to let users know that clicking on the icon will redirect to Manage => Extensions

Code would be something like

diff --git a/administrator/components/com_modules/tmpl/modules/default.php b/administrator/components/com_modules/tmpl/modules/default.php
index 05a53b5..2c46970 100644
--- a/administrator/components/com_modules/tmpl/modules/default.php
+++ b/administrator/components/com_modules/tmpl/modules/default.php
@@ -124,7 +124,11 @@
 							<?php else : ?>
 								<?php // Extension is not enabled, show a message that indicates this. ?>
-								<span class="tbody-icon" title="<?php echo Text::sprintf('COM_MODULES_MSG_MANAGE_EXTENSION_DISABLED', $this->escape($item->name)); ?>">
+								<a class="tbody-icon" href="javascript:void(0);" onClick="location.href='<?php echo Route::_('index.php?option=com_installer&view=manage'); ?>'"
+									aria-labelledby="disabled-<?php echo $this->escape($item->name); ?>">
 									<span class="icon-minus-circle" aria-hidden="true"></span>
-								</span>
+								</a>
+								<div id="disabled-<?php echo $this->escape($item->name); ?>" role="tooltip">
+									<?php echo Text::sprintf('COM_MODULES_MSG_MANAGE_EXTENSION_DISABLED', $this->escape($item->name)); ?>
+								</div>
 							<?php endif; ?>
 						</td>

and result

extensions-manage2

NOTE-UNRELATED ISSUE:

I discovered an important bug when working on this.
It happens when we have one module disabled in the displayed list. That module has no checkbox.
Clicking on blank space in any row ticks the checkbox of the row immediately at the bottom.
Clicking on a checkbox ALSO checks the checkbox below.

sitemodules_tick_issue

Release blocker??

@brianteeman
Copy link
Contributor

The second unrelated issue is already reported

@infograf768
Copy link
Member

Found where it is reported
#30798
but not solved...

@infograf768
Copy link
Member

For the checkboxes, we could simply use #30798 (comment)

@rdeutz rdeutz merged commit 21e51db into joomla:4.0-dev May 29, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 29, 2021
@Quy Quy deleted the manage-extensions branch May 29, 2021 10:54
bembelimen pushed a commit that referenced this pull request Nov 23, 2021
* [4.0] Notification icons (#34226)

* remove method allow parent to run (#34118)

Co-authored-by: Richard Fath <[email protected]>

* [4.0] Changing title to tooltip for template preview (#33292)

* [4.0] Changing title to tooltip fpr template preview

* Update location path in description (#34238)

* Fix patterns field check when field empty and not required (#34124)

* [4.0] Finish transition from CSS classes "label-" to "alert-" for the pre-update check (#34227)

* [4.0] mod_popular with disabled hits (#34257)

* [4.0] mod_popular with disabled hits

When hits are disabled for articles it makes no sense to display the Most Read Articles module in the site or admin as the contents will never be updated.

This PR changes the output of the module so that instead of a list of non-updating articles a message is displayed.

* [4.0] Use MVCFactory to create model (#34092)

* Use MVCFactory to create models

* CS

* Fix row selecting, when module is disabled (#34273)

* Media web service implementation.

* Media web service implementation.

* Fix hard coded adapter name default.

* Fix missing url and tempUrl attributes in single item response.

* Remove useless comments.

* Replace 'PATH' with 'STRING' for cleaning of input parameters.

* Adds required parameter checks and removes global exception handling.

* Adds media web service specific language strings.

* Adds exception handling.

* Adds missing exception handler and fixes wrong path return value after move/rename.

* Use exception message as error title.

* Adds proper FileNotFound exception handling for GET requests.

* Fixes handling of status code.

* Fixes some types, formatting and similar stuff.

* Fixes yet another sloppy typo.

* restore file

* Fix some docs

* Adapter endpoint

* Add plugin to install file

* Add tests

* Update api/components/com_media/src/Helper/MediaHelper.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Helper/MediaHelper.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/MediaModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/MediumModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/View/Media/JsonapiView.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update libraries/src/Error/JsonApi/SaveExceptionHandler.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Controller/MediaController.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Controller/MediaController.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/AdaptersModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update api/components/com_media/src/Model/MediaModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* docs

* Update api/components/com_media/src/View/Media/JsonapiView.php

Co-authored-by: Phil E. Taylor <[email protected]>

* int

* cs

* cs

* more cleanup

* tabs

* restore

* Test all endpoints

* Update api/components/com_media/src/Model/MediaModel.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Hardening

* Tests fix for drone

* Update Api.php

* Create directory in test with correct permissions

* Add more tests and remove static helper

* Update tests/Codeception/api/com_media/MediaCest.php

Co-authored-by: Phil E. Taylor <[email protected]>

* Update tests/Codeception/_support/Helper/Api.php

Co-authored-by: Phil E. Taylor <[email protected]>

* cs

Co-authored-by: Brian Teeman <[email protected]>
Co-authored-by: Phil E. Taylor <[email protected]>
Co-authored-by: Richard Fath <[email protected]>
Co-authored-by: infograf768 <[email protected]>
Co-authored-by: Quy <[email protected]>
Co-authored-by: Ruud <[email protected]>
Co-authored-by: Tuan Pham Ngoc <[email protected]>
Co-authored-by: Fedir Zinchuk <[email protected]>
Co-authored-by: Pieter-Jan de Vries <[email protected]>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants