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.

Sign up for GitHub

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

Jump to bottom

Fix for deselecting item when select_mode == SELECT_ROW #71307

Merged
merged 1 commit into from
Mar 15, 2023
Merged

Fix for deselecting item when select_mode == SELECT_ROW #71307

merged 1 commit into from
Mar 15, 2023

Conversation

eskandrej
Copy link
Contributor

@eskandrej eskandrej commented Jan 13, 2023 •
edited by akien-mga
Loading

Fixes 71306

@eskandrej eskandrej requested a review from a team as a code owner January 13, 2023 01:27
@KoBeWi KoBeWi added this to the 4.0 milestone Jan 13, 2023
marzecdawid
marzecdawid reviewed Jan 14, 2023
View reviewed changes
scene/gui/tree.cpp Outdated Show resolved Hide resolved
@marzecdawid
Copy link
Contributor

As stated here you should combine every commit into one.

marzecdawid
marzecdawid approved these changes Jan 14, 2023
View reviewed changes
Copy link
Contributor

@marzecdawid marzecdawid left a comment

Choose a reason for hiding this comment

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

The code looks good and fixes the problem.

However you should probably create a new PR based on separate branch. Quoting @KoBeWi on that matter after asking if PR based on Master does matter:

yes. PRs on master don't allow you to open a new PR, because every new branch will include master (unless you use some git-fu). Also there is a bug on GitHub, not sure if fixed, but closes master PRs will appear as merged even if they weren't merged. That said, you can't change PRs branch without opening a new one

Follow instructions on this page:
https://docs.godotengine.org/en/stable/community/contributing/pr_workflow.html

@KoBeWi
Copy link
Member

KoBeWi commented Jan 14, 2023 •
edited
Loading

Well, PR on master is ok to keep if it's already opened, but you have to wait until it's merged before you make a new one.
Just for future contributions make a new branch per PR.

@eskandrej
Copy link
Contributor Author

🤦‍♂️ Oh, I understand.

@akien-mga akien-mga modified the milestones: 4.0, 4.1 Feb 17, 2023
akien-mga
akien-mga approved these changes Feb 17, 2023
View reviewed changes
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, but let's wait after the 4.0 release to be safe.

KoBeWi
KoBeWi reviewed Feb 17, 2023
View reviewed changes
scene/gui/tree.cpp Outdated Show resolved Hide resolved
@YuriSizov
Copy link
Contributor

Hey, @eskandrej, can you please rebase your PR against the current master and force-push again? Seems like there were some issues with Godot CPP the last time our checks were run, which blocks this PR from being merged.

@YuriSizov YuriSizov merged commit 7b1b511 into godotengine:master Mar 15, 2023
@YuriSizov
Copy link
Contributor

Thanks! And congrats on your first merged pull request!

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

@KoBeWi KoBeWi KoBeWi approved these changes

@akien-mga akien-mga akien-mga approved these changes

@marzecdawid marzecdawid marzecdawid approved these changes

Assignees
No one assigned
Labels
bug topic:gui
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.

TreeItem.deselect() doesn't work if select_mode is Tree.SELECT_ROW
5 participants
@eskandrej @marzecdawid @KoBeWi @YuriSizov @akien-mga