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] Fix narrow column width in list view of media manager #33945

Merged
merged 4 commits into from
May 19, 2021

Conversation

eopws
Copy link

@eopws eopws commented May 18, 2021

Pull Request for Issue #33943.

Summary of Changes

Fix of the narrow column width in media manager.

Testing Instructions

Go to Content > Media
Click List icon.
Go to sampledata > parks > landscape

Actual result BEFORE applying this Pull Request

Icons column has small width

Expected result AFTER applying this Pull Request

Icons column has enough width

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels May 18, 2021
@Quy
Copy link
Contributor

Quy commented May 18, 2021

I have tested this item ✅ successfully on cfb3f4e

Thank you!


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

@ChristineWk
Copy link

I have tested this item ✅ successfully on cfb3f4e

With Prebuild Package


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

@brianteeman
Copy link
Contributor

Sorry but this is not correct

image

@eopws
Copy link
Author

eopws commented May 18, 2021

@brianteeman
Is that because of incorrect line break?
The problem from the issue is caused by lack of width in icon column, and I know two solutions:

  • The one proposed in the PR
  • Set min-width property to icon cell

If you consider line break a bad solution I can replace it with min-width property.

@brianteeman
Copy link
Contributor

The size of the other columns is way too big. you really dont want to wrap a filename unless you absolutely have to

image

Better to set some more sensible column widths such as


.media-browser-table .type {
    width: 5%;
}

.media-browser-table .size {
    width: 10%;
    text-align: right;
}

.media-browser-table .dimension {
    width: 15%
}

.media-browser-table .created,.media-browser-table .modified {
    width: 15%;
}

@eopws
Copy link
Author

eopws commented May 18, 2021

@brianteeman

Better to set some more sensible column widths such as

But we are still gonna have the problem when the file name is too big. Try to add symbols to a filename in devtools.
I think it's cool to change column widths as you proposed and add the word-break: break-all; property to 'name' columns. Then if the filename will be very big the layout would not be broken.

@brianteeman
Copy link
Contributor

But we are still gonna have the problem when the file name is too big.

Always going to happen but we can reduce the times when it will happen by setting sensible column widths in the beginning - especially when the width of the data in the column is never going to be that wide

@eopws
Copy link
Author

eopws commented May 18, 2021

Please, test new patch.

@ChristineWk
Copy link

I have tested this item ✅ successfully on b1b17be

Installed the new Prebuild Package. It's much better now.


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

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on b1b17be


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

@richard67
Copy link
Member

RTC


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

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

Quy commented May 18, 2021

The first column should align.

33945

@richard67 richard67 removed the RTC This Pull Request is Ready To Commit label May 18, 2021
@joomla-cms-bot joomla-cms-bot removed this from the Joomla 4.0 milestone May 18, 2021
@richard67
Copy link
Member

Back to pending, see comment #33945 (comment) .


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

@richard67 richard67 added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label May 18, 2021
Doing this in order to align the table columns with the "check all" button.
Here we set 'width' and 'min-width' properties to the type column and remove the 'width' property from the name column.
By doing this the type column will always have size of 50px (as the "check all" button) and the name column will take all free place.
@eopws
Copy link
Author

eopws commented May 19, 2021

The patch works on my machine. Please, test it too.

@richard67 richard67 removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label May 19, 2021
@ceford
Copy link
Contributor

ceford commented May 19, 2021

I have tested this item ✅ successfully on 0d9143d

Better that it was. But why does the icon cell have a bottom border but the rest of the row does not?


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

@eopws
Copy link
Author

eopws commented May 19, 2021

But why does the icon cell have a bottom border but the rest of the row does not?

What do you mean? Please, send screenshot.

@ChristineWk
Copy link

I'm not sure & see it now in this way:

screen shot 2021-05-19 at 09 31 01

or what do you mean @ceford?


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

@eopws
Copy link
Author

eopws commented May 19, 2021

#33945 (comment)
I can fix it, but it is out of the PR's topic.
Also it was before applying the patch too.

@ChristineWk
Copy link

I have tested this item ✅ successfully on 0d9143d

@eopws OK. Don't remember the previous last line/border :-)


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

@richard67
Copy link
Member

RTC


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

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

Quy commented May 19, 2021

In Firefox, it is a pixel off to the left. In Chrome, it is fine.

The toggle checkbox is 50px.

Please see if you can use the same value and look good in both browsers.

@eopws
Copy link
Author

eopws commented May 19, 2021

In Firefox, it is a pixel off to the left. In Chrome, it is fine.

The toggle checkbox is 50px.

Please see if you can use the same value and look good in both browsers.

In Chrome, it is a pixel off to the right, if set min-width equals to 50px.
But in Chrome it doesn't seem to be critical while it looks bad in Firefox.

@Quy Quy merged commit a602506 into joomla:4.0-dev May 19, 2021
@Quy
Copy link
Contributor

Quy commented May 19, 2021

Merging as this is better than before. Thank you!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 19, 2021
@eopws eopws deleted the Minor-fix-in-media-com-layout branch May 23, 2021 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

8 participants