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

Fix : Hides hidden files and folder in modal (beginning with a dot) #37597

Merged
merged 8 commits into from Apr 20, 2023

Conversation

Jerome-Herbinet
Copy link
Member

Signed-off-by: Jérôme Herbinet 33763786+Jerome-Herbinet@users.noreply.github.com

Summary

Fixes the mentioned problem in this modal (IMO this is a problem) :

2023-04-05_13-20_1

Checklist

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
@szaimen szaimen added the 3. to review Waiting for reviews label Apr 5, 2023
@szaimen szaimen modified the milestones: Nextcloud 25.0.7, Nextcloud 27 Apr 5, 2023
@szaimen szaimen requested review from JuliaKirschenheuter, a team, artonge, Pytal and szaimen and removed request for a team April 5, 2023 11:27
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Works, but a bit of a hack, no?
What would a solution in js look like?

@Jerome-Herbinet
Copy link
Member Author

Works, but a bit of a hack, no? What would a solution in js look like?

@artonge, I have no skill on JS, so I don't know what could be the solution.

@artonge
Copy link
Contributor

artonge commented Apr 19, 2023

I dug a little, and looks like a condition already exist, but it is wrong in my opinion.

server/core/src/OC/dialogs.js

Lines 1174 to 1179 in 9db3305

const showHiddenInput = document.getElementById('showHiddenFiles')
const showHidden = showHiddenInput === null || showHiddenInput.value === "1"
if (!showHidden) {
files = files.filter(function(file) {
return !file.name.startsWith('.')
})

The code is searching for #showHiddenFiles, but no component with this id is created for the settings page. It is only created for files. The code then default to showing hidden files, which is unexpected, it should be the opposite.

Replacing the code by the following works. @Jerome-Herbinet can you handle that? I don't think I can push to your branch.

		const showHiddenInput = document.getElementById('showHiddenFiles')
		if (showHiddenInput?.value !== "1") {
			files = files.filter(function (file) {
				return !file.name.startsWith('.')
			})
		}

Commit: 1fa58be

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
@Jerome-Herbinet
Copy link
Member Author

I dug a little, and looks like a condition already exist, but it is wrong in my opinion.

server/core/src/OC/dialogs.js

Lines 1174 to 1179 in 9db3305

const showHiddenInput = document.getElementById('showHiddenFiles')
const showHidden = showHiddenInput === null || showHiddenInput.value === "1"
if (!showHidden) {
files = files.filter(function(file) {
return !file.name.startsWith('.')
})

The code is searching for #showHiddenFiles, but no component with this id is created for the settings page. It is only created for files. The code then default to showing hidden files, which is unexpected, it should be the opposite.

Replacing the code by the following works. @Jerome-Herbinet can you handle that? I don't think I can push to your branch.

		const showHiddenInput = document.getElementById('showHiddenFiles')
		if (showHiddenInput?.value !== "1") {
			files = files.filter(function (file) {
				return !file.name.startsWith('.')
			})
		}

Commit: 1fa58be

Done @artonge :-)

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
@Jerome-Herbinet
Copy link
Member Author

I dug a little, and looks like a condition already exist, but it is wrong in my opinion.

server/core/src/OC/dialogs.js

Lines 1174 to 1179 in 9db3305

const showHiddenInput = document.getElementById('showHiddenFiles')
const showHidden = showHiddenInput === null || showHiddenInput.value === "1"
if (!showHidden) {
files = files.filter(function(file) {
return !file.name.startsWith('.')
})

The code is searching for #showHiddenFiles, but no component with this id is created for the settings page. It is only created for files. The code then default to showing hidden files, which is unexpected, it should be the opposite.
Replacing the code by the following works. @Jerome-Herbinet can you handle that? I don't think I can push to your branch.

		const showHiddenInput = document.getElementById('showHiddenFiles')
		if (showHiddenInput?.value !== "1") {
			files = files.filter(function (file) {
				return !file.name.startsWith('.')
			})
		}

Commit: 1fa58be

Done @artonge :-)

Can you test on your side @artonge ?

@artonge
Copy link
Contributor

artonge commented Apr 19, 2023

Can you remove the original change? It should not be needed anymore

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
@Jerome-Herbinet
Copy link
Member Author

Can you remove the original change? It should not be needed anymore

Done + npm run sass @artonge

@artonge
Copy link
Contributor

artonge commented Apr 20, 2023

Perfect 👌
CI is complaining about the compiled files, your local node_modules might need an update with npm ci, then compile again.

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
@Jerome-Herbinet
Copy link
Member Author

Perfect ok_hand CI is complaining about the compiled files, your local node_modules might need an update with npm ci, then compile again.

Done

@artonge artonge requested review from skjnldsv and a team April 20, 2023 10:24
@artonge artonge merged commit c909e92 into nextcloud:master Apr 20, 2023
36 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants