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

Fix: Adding thumbs file names in Media Manager #16769

Merged
merged 5 commits into from Jun 21, 2017

Conversation

Projects
None yet
5 participants
@infograf768
Member

infograf768 commented Jun 19, 2017

Pull Request for Issue #16710

The name of the image files do not display in Isis Media Manager.

After patch, one should get in LTR

screen shot 2017-06-19 at 11 29 19

and in RTL
screen shot 2017-06-19 at 11 30 12

For RTL, as it was impossible to keep the magnifiying glass to the right and that anyway a file name should be LTR, I have put it to the left as in LTR. It is not an issue imho in that specific case to move it.

@bertmert @franz-wohlkoenig
Getting the image title in the Preview modal title is not easy. It requires some JS to append it.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 19, 2017

Can you screw on Visibility?
bildschirmfoto 2017-06-19 um 12 12 42

Can you screw on Visibility?
bildschirmfoto 2017-06-19 um 12 12 42

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jun 20, 2017

Member

Do you mean to get this?

screen shot 2017-06-20 at 08 00 32

Member

infograf768 commented Jun 20, 2017

Do you mean to get this?

screen shot 2017-06-20 at 08 00 32

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 20, 2017

@infograf768 yes, mean to read Filenames better.

@infograf768 yes, mean to read Filenames better.

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jun 20, 2017

Member

Here it is. One should now get this:
screen shot 2017-06-20 at 11 45 01

Please test again.

Member

infograf768 commented Jun 20, 2017

Here it is. One should now get this:
screen shot 2017-06-20 at 11 45 01

Please test again.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 20, 2017

RTL (Persian): Should the Symbol be on right Side?
1

RTL (Persian): Should the Symbol be on right Side?
1

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jun 20, 2017

Member

RTL (Persian): Should the Symbol be on right Side?

Impossible. I spoke about that in my original description.

For RTL, as it was impossible to keep the magnifiying glass to the right and that anyway a file name should be LTR, I have put it to the left as in LTR. It is not an issue imho in that specific case to move it.

Member

infograf768 commented Jun 20, 2017

RTL (Persian): Should the Symbol be on right Side?

Impossible. I spoke about that in my original description.

For RTL, as it was impossible to keep the magnifiying glass to the right and that anyway a file name should be LTR, I have put it to the left as in LTR. It is not an issue imho in that specific case to move it.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 20, 2017

I have tested this item successfully on 431263e


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

I have tested this item successfully on 431263e


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

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 20, 2017

Contributor

Text is on 2 lines.
headers

Text font size is larger than folder font size.
folders

Contributor

Quy commented Jun 20, 2017

Text is on 2 lines.
headers

Text font size is larger than folder font size.
folders

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 20, 2017

@Quy which Browser, OS? I'm using Macos+Firefox, Result see Screenshot above.

@Quy which Browser, OS? I'm using Macos+Firefox, Result see Screenshot above.

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 20, 2017

Contributor

Firefox 54.0 on Windows 10 Home

Contributor

Quy commented Jun 20, 2017

Firefox 54.0 on Windows 10 Home

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 20, 2017

Contributor

If there is a hyphen in the file name, then it will wrap 2 lines.

Contributor

Quy commented Jun 20, 2017

If there is a hyphen in the file name, then it will wrap 2 lines.

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jun 20, 2017

Member

I have added a nowrap class to solve the issue with the hyphen.
Please @Quy @franz-wohlkoenig
test again.

Member

infograf768 commented Jun 20, 2017

I have added a nowrap class to solve the issue with the hyphen.
Please @Quy @franz-wohlkoenig
test again.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 20, 2017

I have tested this item successfully on bba284d

Test on MacOs Firefox, Chrome, Safari (all latest)


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

I have tested this item successfully on bba284d

Test on MacOs Firefox, Chrome, Safari (all latest)


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

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 20, 2017

Contributor

Will the folder text size be the same as the file text size?

Contributor

Quy commented Jun 20, 2017

Will the folder text size be the same as the file text size?

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jun 20, 2017

Member

Changed file text size to match folder and extended truncate to 16 letters.

To get this:
screen shot 2017-06-20 at 16 13 03

Member

infograf768 commented Jun 20, 2017

Changed file text size to match folder and extended truncate to 16 letters.

To get this:
screen shot 2017-06-20 at 16 13 03

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 20, 2017

Contributor

Some text are touching the right edge.

edge

Contributor

Quy commented Jun 20, 2017

Some text are touching the right edge.

edge

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 20, 2017

Contributor

Here is another solution using the truncate class. Also, moved the text out of the span tag.

 		<div class="imgPreview nowrap small">
			<a href="<?php echo COM_MEDIA_BASEURL, '/', $img->path_relative; ?>" title="<?php echo $img->name; ?>" class="preview truncate">
				<span class="icon-search" aria-hidden="true"></span><?php echo $img->name; ?>
			</a>
		</div>

With your solution, there is room to display the g but it is truncated based on # of characters.
truncate

Contributor

Quy commented Jun 20, 2017

Here is another solution using the truncate class. Also, moved the text out of the span tag.

 		<div class="imgPreview nowrap small">
			<a href="<?php echo COM_MEDIA_BASEURL, '/', $img->path_relative; ?>" title="<?php echo $img->name; ?>" class="preview truncate">
				<span class="icon-search" aria-hidden="true"></span><?php echo $img->name; ?>
			</a>
		</div>

With your solution, there is room to display the g but it is truncated based on # of characters.
truncate

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jun 21, 2017

Member

Thanks @Quy. Done.

Don't forget to also test in rtl.

Member

infograf768 commented Jun 21, 2017

Thanks @Quy. Done.

Don't forget to also test in rtl.

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 21, 2017

I have tested this item successfully on 9a93825

Test LTR and also in RTL (Persian).


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

I have tested this item successfully on 9a93825

Test LTR and also in RTL (Persian).


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

@infograf768 infograf768 added this to the Joomla 3.7.4 milestone Jun 21, 2017

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 21, 2017

Contributor

Sorry to nitpick. See the arrow. The horizontal lines don't line up. The line in the file box is slightly higher than the one in the folder box.

after-border

Contributor

Quy commented Jun 21, 2017

Sorry to nitpick. See the arrow. The horizontal lines don't line up. The line in the file box is slightly higher than the one in the folder box.

after-border

@infograf768

This comment has been minimized.

Show comment
Hide comment
@infograf768

infograf768 Jun 21, 2017

Member

@Quy I guess we will survive with this.

Member

infograf768 commented Jun 21, 2017

@Quy I guess we will survive with this.

@Quy

This comment has been minimized.

Show comment
Hide comment
@Quy

Quy Jun 21, 2017

Contributor

I have tested this item successfully on 9a93825


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

Contributor

Quy commented Jun 21, 2017

I have tested this item successfully on 9a93825


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.7.4 milestone Jun 21, 2017

@franz-wohlkoenig

This comment has been minimized.

Show comment
Hide comment
@franz-wohlkoenig

franz-wohlkoenig Jun 21, 2017

RTC after two successful tests.

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC label Jun 21, 2017

@infograf768 infograf768 added this to the Joomla 3.7.4 milestone Jun 21, 2017

@rdeutz rdeutz modified the milestones: Joomla 3.7.3, Joomla 3.7.4 Jun 21, 2017

@rdeutz rdeutz merged commit 61d2ed1 into joomla:staging Jun 21, 2017

5 checks passed

JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!

@joomla-cms-bot joomla-cms-bot removed the RTC label Jun 21, 2017

@infograf768 infograf768 deleted the infograf768:media_image_names branch Jun 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment