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] Remove module description popover #24634

Merged
merged 3 commits into from
Apr 22, 2019

Conversation

brianteeman
Copy link
Contributor

When you go to select a NEW module you have a list of all the modules with a very short 90 character truncated description.

If you hover over that description you get a popover with a short 200 character truncated description. This can result in a popover that is identical to the text it is supposed to be describing

This popover is not accessible - I doubt even most sighted users will know that it even exists as there is no visual indicator that it exists either.

The original reason for the 90 character truncation was back in the day that this was a modal and there was limited space. That is no longer true but we do need to have some form of truncation as some 3pd write essays for the description ;) so I chose the 200 character limit

This PR removes the popover and changes the displayed description to the 200 character version

I am including the screenshots as I doubt most people even knew the popover existed

Before

image

After

image

When you go to select a NEW module you have a list of all the modules with a very short 90 character truncated description.

If you hover over that description you get a popover with a short 200 character truncated description.

This popover is not accessible - I doubt even most sighted users will know that it even exists as there is no visual indicator that it exists either.

The original reason for the 90 character truncation was back in the day that this was a modal and there was limited space. That is no longer true  but we do need to have some form of truncation as some 3pd write essays for the description ;)

This PR removes the popover and changes the displayed description to the 200 character version
@@ -25,17 +24,15 @@
<?php $link = 'index.php?option=com_modules&task=module.add&eid=' . $item->extension_id; ?>
<?php $name = $this->escape($item->name); ?>
<?php $desc = HTMLHelper::_('string.truncate', $this->escape(strip_tags($item->desc)), 200); ?>
<?php $short_desc = HTMLHelper::_('string.truncate', $this->escape(strip_tags($item->desc)), 90); ?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove spaces before =.

@Quy
Copy link
Contributor

Quy commented Apr 18, 2019

I know this has nothing to do with this PR. Please confirm that the link should be on the right side.

@infograf768

rtl

@brianteeman
Copy link
Contributor Author

Strange. It was on the right in my test setting the XML value to right

@brianteeman
Copy link
Contributor Author

I confirm your results with the farsi language
BUT
If you set <rtl>1</rtl>
in the english language it works correctly

I am assuming there is an issue with this line of code <?php if ($document->direction != 'rtl') : ?>
but that is unrelated to this pr

@Quy
Copy link
Contributor

Quy commented Apr 18, 2019

I have tested this item ✅ successfully on 8fc1849


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

1 similar comment
@richard67
Copy link
Member

I have tested this item ✅ successfully on 8fc1849


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

@ghost
Copy link

ghost commented Apr 21, 2019

Status "Ready To Commit".

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 21, 2019
@wilsonge
Copy link
Contributor

So is there any way now of seeing the untruncated description in this view?

@brianteeman
Copy link
Contributor Author

Same as before. In the module itself.

In the list view before we showed 90 characters and 200 on hover
Now we show 200.

@wilsonge
Copy link
Contributor

Ahh sorry I missed we were only showing 200 in the popover

@wilsonge wilsonge merged commit bd9875e into joomla:4.0-dev Apr 22, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Apr 22, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 22, 2019
@wilsonge
Copy link
Contributor

Thanks!

@brianteeman
Copy link
Contributor Author

No problem. I was surprised at the limit in the popover too.

@brianteeman brianteeman deleted the module_popover branch April 22, 2019 22:44
@infograf768
Copy link
Member

@Quy
Yes, the link should be on the right side.
We have the same error in 3.9.x
=> No use in this case to have a specific order for rtl.

diff --git a/administrator/components/com_modules/tmpl/select/default.php b/administrator/components/com_modules/tmpl/select/default.php
index 5157566..86a26df 100644
--- a/administrator/components/com_modules/tmpl/select/default.php
+++ b/administrator/components/com_modules/tmpl/select/default.php
@@ -25,16 +25,8 @@
 	<?php $name = $this->escape($item->name); ?>
 	<?php $desc = HTMLHelper::_('string.truncate', $this->escape(strip_tags($item->desc)), 200); ?>
-
-	<?php if ($document->direction != 'rtl') : ?>
 	<li class="list-group-item">
 		<a href="<?php echo Route::_($link); ?>" class="mr-2"><strong><?php echo $name; ?></strong></a>
 		<small><?php echo $desc; ?></small>
 	</li>
-	<?php else : ?>
-	<li class="list-group-item">
-		<small><?php echo $desc; ?></small>
-		<a href="<?php echo Route::_($link); ?>" class="mr-2"><strong><?php echo $name; ?></strong></a>
-	</li>
-	<?php endif; ?>
 <?php endforeach; ?>
 </ul>

@infograf768
Copy link
Member

infograf768 commented Apr 23, 2019

Code was OK in 2012. ;)

fancyFranci pushed a commit to fancyFranci/joomla-cms that referenced this pull request Apr 24, 2019
When you go to select a NEW module you have a list of all the modules with a very short 90 character truncated description.

If you hover over that description you get a popover with a short 200 character truncated description.

This popover is not accessible - I doubt even most sighted users will know that it even exists as there is no visual indicator that it exists either.

The original reason for the 90 character truncation was back in the day that this was a modal and there was limited space. That is no longer true  but we do need to have some form of truncation as some 3pd write essays for the description ;)

This PR removes the popover and changes the displayed description to the 200 character version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants