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

Show file names in remove files confirmation dialog #98539

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

pafuent
Copy link
Contributor

@pafuent pafuent commented Oct 26, 2024

Closes #85261
Supersedes #85290

Thanks to @jsjtxietian that implemented most of the code. I just rebased the original PR and did some tweaks to adjust ItemList and Tree sizes.

MultipleFiles
Directories
MultipleFiles

@pafuent pafuent requested a review from a team as a code owner October 26, 2024 01:56
@timothyqiu timothyqiu added this to the 4.4 milestone Oct 26, 2024
@pafuent pafuent force-pushed the show_filename_when_delete branch from 1863712 to 200541b Compare November 13, 2024 11:18
@pafuent
Copy link
Contributor Author

pafuent commented Nov 13, 2024

Can I get a review?

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Seems good to me.

Copy link
Contributor

@jsjtxietian jsjtxietian left a comment

Choose a reason for hiding this comment

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

LGTM

@KoBeWi
Copy link
Member

KoBeWi commented Nov 13, 2024

The dialog looks confusing when deleted file is a dependency:
image
There should be some label between the 2 boxes.

Also when deleting single file, the item list is scrollable by a pixel:
image

@pafuent
Copy link
Contributor Author

pafuent commented Nov 15, 2024

Also when deleting single file, the item list is scrollable by a pixel

@KoBeWi I tried a lot of different things trying to know the size of one tree item to set the minimum size of the tree. My plan was to have the size of 5 items as max (for less items just set it to see all the items)
The best approximation are this lines:

if (tree_item_count > 0) {
	const uint32_t multiplier = MIN(tree_item_count, 5);
	owners->set_custom_minimum_size(Size2(0, 35 * multiplier) * EDSCALE);
}

Is there a better way to know the exact size of a tree item?
(sadly my knowledge on Godot GUI is limited)

@KoBeWi
Copy link
Member

KoBeWi commented Nov 15, 2024

There is get_minimum_size() method in TreeItem.

@pafuent pafuent force-pushed the show_filename_when_delete branch from 200541b to ae1cdef Compare November 18, 2024 04:02
@pafuent
Copy link
Contributor Author

pafuent commented Nov 18, 2024

After trying several methods to set a custom minimum size, I decided to just use one with a fixed value to keep the code simpler.

Closes godotengine#85261

Co-authored-by: jsjtxietian <jsjtxietian@outlook.com>
@pafuent pafuent force-pushed the show_filename_when_delete branch from ae1cdef to 56d01fb Compare November 18, 2024 04:04
@Repiteo Repiteo merged commit ce4674a into godotengine:master Nov 27, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 27, 2024

Thanks!

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

Successfully merging this pull request may close these issues.

"File remove" confirmation has no file name.
6 participants