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

Some context menu changes for the main tree view #1973

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

LukeUniverse
Copy link

Added a few options to the right click context menu for the TreeView on the left hand side of the screen.
New items I added:

Move up to {Previous Section}
Move down to {Next Section}

Collapse all
Collapse {Current Section}

Add: {CurrentSection}

Where Current/Previous/Next Section is replaced with the text of the that section such as "Sounds" "Tile sets" "Sprites", etc etc.

I also rearranged the MenuItem_*** methods to be alphabetically ordered. Which has created some visual clutter in this Pr which I probably should have avoided >.< oops, hah. First time trying to do a PR on github, so I guess I'm learning. hah

Discussed with some people in the discord server, and they encouraged me to make a PR, so uh, this is my attempt at that.

Adds a few things to the right click context menu of the treeview:

New items I added:
<Seperator/>
Move up to {Previous Section}
Move down to {Next Section}
<Seperator/>
Collapse all
Collapse {Current Section}
<Seperator/>
Add: {CurrentSection}

Where Current/Previous/Next Section is replaced with the text of the that section such as "Sounds" "Tile sets" "Sprites", etc etc.
oops this should have been with the previous check in
Copy link
Contributor

@Miepee Miepee left a comment

Choose a reason for hiding this comment

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

Heads up: for GUI changes, screenshots are always nice to have :)

Also for the future, if you're doing just visual ordering, make these in seperate commits please so that it's easier to review

Comment on lines +263 to +264
<MenuItem Header="Move up to" Click="MenuItem_MoveUpTo_Click"/>
<MenuItem Header="Move down to" Click="MenuItem_MoveDownTo_Click"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

What are these supposed to do?

if (Highlighted is UndertaleObject obj)
DeleteItem(obj);
}
//this 99% copied from the the Add_Click, so could probably have the fuctionality of it (and above) combined into one method that then gets called from both events)
Copy link
Contributor

Choose a reason for hiding this comment

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

please don't copy paste for 90%

Also because I cant find it right now, whats the difference between Add and AddType ?

Comment on lines +2082 to +2084
foreach (TreeViewItem treeItem in (MainTree.Items[0] as TreeViewItem).Items)
if (treeItem.Items.Contains(MainTree.SelectedItem))
indexOfCurrentSelectionParent = (MainTree.Items[0] as TreeViewItem).Items.IndexOf(treeItem);
Copy link
Contributor

Choose a reason for hiding this comment

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

please refer to our code style
use braces

Comment on lines +2188 to +2190
foreach (TreeViewItem item in (MainTree.Items[0] as TreeViewItem).Items)
if (item.Items.Contains(MainTree.SelectedItem))
item.IsExpanded = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

braces

Comment on lines +2182 to +2183
foreach (TreeViewItem item in (MainTree.Items[0] as TreeViewItem).Items)
item.IsExpanded = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

braces

Comment on lines +2197 to +2198
//LMM: Regarding the text changes, and visibility changes here... there is most likely better ways to do this, I'm not super familair with XAML though, and eh, this worked for me.
//other contributors could polish this up if need be :)
Copy link
Contributor

Choose a reason for hiding this comment

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

code style: please have a space after the // and start with an uppercase letter

What does LMM mean?

Also CC @VladiStep cause you probably know more xaml than anyone else on the dev team.

Comment on lines +2353 to +2356
foreach (TreeViewItem item in (MainTree.Items[0] as TreeViewItem).Items)
if (item.Items.Contains(MainTree.SelectedItem))
indexOfCurrentSelectionParent = (MainTree.Items[0] as TreeViewItem).Items.IndexOf(item);

Copy link
Contributor

Choose a reason for hiding this comment

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

braces

Comment on lines +2363 to +2365
foreach (TreeViewItem treeItem in (MainTree.Items[0] as TreeViewItem).Items)
if (treeItem.Items.Contains(MainTree.SelectedItem))
indexOfCurrentSelectionParent = (MainTree.Items[0] as TreeViewItem).Items.IndexOf(treeItem);
Copy link
Contributor

Choose a reason for hiding this comment

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

braces

item.IsExpanded = false;
}

private void MenuItem_ContextMenuOpened(object sender, RoutedEventArgs e)
Copy link
Contributor

Choose a reason for hiding this comment

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

please double check codestyle in this whole function, not gonna point it out in every instance.

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.

2 participants