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

Bugfix: Clicking in the middle of the row in the file list downloads the file #39361

Merged
merged 2 commits into from
Oct 19, 2021

Conversation

AlexAndBear
Copy link

@AlexAndBear AlexAndBear commented Oct 18, 2021

Description

This change addresses the issue if the user clicks in on a row in the file list,
the file gets downloaded or open with the default viewer.
This was not intended, the download or default opening should only happen if
the user clicks directly on the file name.
Problems with mobile devices, where the file name was too long to display,
has been also solved.

Related Issue

Motivation and Context

How Has This Been Tested?

  • MS Edge
  • IE 11
  • Google Chrome
  • Firefox
  • Safari

Screenshots (if appropriate):

Mobile view

  • Before
    image

  • After
    image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@AlexAndBear AlexAndBear changed the title Remove faulty max width Remove faulty width Oct 18, 2021
@AlexAndBear AlexAndBear changed the title Remove faulty width Remove faulty width from files list tr, enhances mobile view and click of empty space opens the right sightbar menu Oct 18, 2021
@AlexAndBear AlexAndBear changed the title Remove faulty width from files list tr, enhances mobile view and click of empty space opens the right sightbar menu Remove faulty width from files list tr, enhances mobile view also click in empty space opens the right sightbar menu Oct 18, 2021
@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline webUIFavorites-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/33071/127/1

@AlexAndBear AlexAndBear changed the title Remove faulty width from files list tr, enhances mobile view also click in empty space opens the right sightbar menu Bugfix: Clicking in the middle of the row in the file list downloads the file Oct 18, 2021
@owncloud owncloud deleted a comment from update-docs bot Oct 18, 2021
@AlexAndBear AlexAndBear marked this pull request as ready for review October 18, 2021 11:30
@AlexAndBear AlexAndBear self-assigned this Oct 18, 2021
JammingBen
JammingBen previously approved these changes Oct 18, 2021
@jvillafanez
Copy link
Member

Some additional changes to consider:

diff --git a/apps/files/css/files.css b/apps/files/css/files.css
index cdacd5408b..5a58de083d 100644
--- a/apps/files/css/files.css
+++ b/apps/files/css/files.css
@@ -89,10 +89,6 @@
 	min-height: 0;
 }
 
-.app-files #app-content {
-	overflow-x: hidden;
-}
-
 /* icons for sidebar */
 .nav-icon-files {
 	background-image: url("../img/folder.svg");
@@ -808,7 +804,6 @@ html.ie8 .column-mtime .selectedActions {
 	filter: alpha(opacity=30);
 	opacity: 0.3;
 	/* add whitespace to bottom of files list to correctly show dropdowns */
-	height: 300px;
 }
 
 .summary,
@@ -822,6 +817,7 @@ table tr.summary td {
 	border-bottom: none;
 	vertical-align: top;
 	padding-top: 20px;
+	padding-bottom: 20px;
 }
 
 .summary .info {
diff --git a/core/css/apps.css b/core/css/apps.css
index 6b5c8fec77..32d2e34fce 100644
--- a/core/css/apps.css
+++ b/core/css/apps.css
@@ -471,7 +471,7 @@
 }
 
 #app-content.with-app-sidebar {
-	margin-right: 27%;
+	margin-right: max(27%, 300px);
 }
 
 #app-sidebar.disappear {

I've only checked with the files app (just with firefox), and it looks better with those changes. The comment about the dropdown seems partially obsolete: the window is scrolled down to show all the items. Again, I've just checked with firefox, and some stuff might not be compatible with all the browser (such as IE), so we still need to verify the proposed changes.

@AlexAndBear
Copy link
Author

@jvillafanez Yes, there is a lot of unused and mis-commented stuff in there, I don't want to touch any of them

@@ -68,17 +68,6 @@
top: 44px;
}

/* make sure there's enough room for the file actions */
#body-user #filestable {
Copy link
Author

Choose a reason for hiding this comment

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

Note this has been ignored in all modern browsers by

@media only screen and (max-width: 768px)
#body-user #filestable {
    min-width: initial !important;
}

but not on IE11 which was responsible for UI glitches

@AlexAndBear
Copy link
Author

Checked with different browsers , LGTM

@sonarcloud
Copy link

sonarcloud bot commented Oct 19, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@AlexAndBear AlexAndBear merged commit 7790979 into master Oct 19, 2021
@delete-merged-branch delete-merged-branch bot deleted the issues/39329 branch October 19, 2021 08:16
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.

Clicking in the middle of the row in the file list downloads the file instead of showing the details
4 participants