Skip to content

Conversation

@juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Apr 19, 2018

This PR moves the sizing of the filename column to a flexbox based layout. There is a small hack needed (defining a width of 0) to make sure that the auto table layout algorithm doesn't use the text width as minimum width for the table cell.

Besides that this approach seems a lot cleaner than before, as we don't need any fixed width rules for different browser sizes, that might break on certain ui changes. It is basically indipendent of the other table cells and the width of the file icon/file actions.

I've tested this on Chrome and Firefox, but my Windows VM doesn't boot right now so I would appreciate if someone could check IE/Edge.

Fixes #8940

files app

image

image

public shared folder

image

image

@juliusknorr juliusknorr added bug design Design, UI, UX, etc. 3. to review Waiting for reviews high labels Apr 19, 2018
@juliusknorr juliusknorr added this to the Nextcloud 14 milestone Apr 19, 2018
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@codecov
Copy link

codecov bot commented Apr 19, 2018

Codecov Report

Merging #9248 into master will increase coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #9248      +/-   ##
============================================
+ Coverage      51.9%   51.94%   +0.04%     
- Complexity    25365    25383      +18     
============================================
  Files          1607     1608       +1     
  Lines         95330    95424      +94     
  Branches       1394     1394              
============================================
+ Hits          49477    49571      +94     
  Misses        45853    45853
Impacted Files Coverage Δ Complexity Δ
...s/federatedfilesharing/lib/AppInfo/Application.php 63.63% <0%> (-1.18%) 5% <0%> (ø)
lib/private/Group/Database.php 99.27% <0%> (-0.73%) 26% <0%> (ø)
lib/private/Installer.php 58.39% <0%> (-0.16%) 71% <0%> (-2%)
...iles_sharing/lib/Controller/ShareAPIController.php 68.03% <0%> (-0.15%) 160% <0%> (ø)
apps/user_ldap/lib/Access.php 36.36% <0%> (ø) 326% <0%> (ø) ⬇️
...les_external/lib/Service/LegacyStoragesService.php 0% <0%> (ø) 24% <0%> (ø) ⬇️
lib/private/Files/ObjectStore/SwiftFactory.php 0% <0%> (ø) 38% <0%> (+1%) ⬆️
apps/user_ldap/ajax/wizard.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
config/config.sample.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
lib/public/Group/Backend/ABackend.php 88.23% <0%> (ø) 8% <0%> (?)
... and 6 more

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Flex! 💪

@MorrisJobke
Copy link
Member

Sadly it doesn't work in Safari (but works in chrome): 😢

bildschirmfoto 2018-04-20 um 10 48 54

@skjnldsv
Copy link
Member

Arf, damn safari again :'(

@juliusknorr
Copy link
Member Author

😞 Let me see if I can get my hands on a mac over the weekend.

@juliusknorr juliusknorr added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 20, 2018
@johnyb0y
Copy link

Thanks for fixing this! Do you think your pull request will make it into 13.0.2 (considering RC1 is already released)?

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
Copy link
Member Author

@MorrisJobke Can you test again? It now works for me in Safari as well, but I could only test Safari 10, not sure if the newest will behave the same. I also tested it successfully in IE11 and Edge.

@juliusknorr juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 22, 2018
@MorrisJobke
Copy link
Member

@MorrisJobke Can you test again? It now works for me in Safari as well, but I could only test Safari 10, not sure if the newest will behave the same. I also tested it successfully in IE11 and Edge.

Doesn't work here :( Safari Version 11.1 (13605.1.33.1.2)

@nickvergessen
Copy link
Member

Can you test again?

Btw, there was no push, so maybe it would work with your local changes?!

@jancborchardt
Copy link
Member

Actually good question if this is backportable. Seems lots of changes with breaking potential, but it's also an essential fix, so not sure. @MorrisJobke @juuliushaertl?

@MorrisJobke
Copy link
Member

Actually good question if this is backportable. Seems lots of changes with breaking potential, but it's also an essential fix, so not sure. @MorrisJobke @juuliushaertl?

Let's first get a working version and then check out if we should backport or not.

@juliusknorr
Copy link
Member Author

Btw, there was no push, so maybe it would work with your local changes?!

Aw, sorry, I forgot to push the changes. 🙈 @MorrisJobke Can you please give it another try?

@MorrisJobke
Copy link
Member

Aw, sorry, I forgot to push the changes. 🙈 @MorrisJobke Can you please give it another try?

Yeeees! Works now 😄

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish backport-request and removed 3. to review Waiting for reviews labels Apr 24, 2018
@rullzer rullzer merged commit 0dc0193 into master Apr 24, 2018
@rullzer rullzer deleted the fix-files-ellipsis branch April 24, 2018 18:36
@MorrisJobke
Copy link
Member

@juliushaertl Please backport to stable13 but only for 13.0.3 and not for 13.0.2 anymore ;)

@juliusknorr
Copy link
Member Author

backport in #9344

@johnyb0y
Copy link

johnyb0y commented Jun 9, 2018

@juliushaertl So I just updated to 13.0.3 and it's still happening :/ No Change at all.

iPhone 7, Safari, latest iOS Version.

@MorrisJobke
Copy link
Member

@johnyb0y could you please open a new ticket and reference this one? Thanks

@johnyb0y
Copy link

johnyb0y commented Jun 10, 2018

@MorrisJobke Of course, but wouldn't it be better to reopen my original issue, since it's the still the same problem and nothing has changed?
#8940

EDIT: Nevermind, #9817

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

Labels

4. to release Ready to be released and/or waiting for tests to finish bug design Design, UI, UX, etc. high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

On Mobile: Bad utilization of space for Folder/File Names on Website

8 participants