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

SymbolsView not adding scrollbar #192

Closed
rohrlich opened this issue Mar 17, 2020 · 8 comments
Closed

SymbolsView not adding scrollbar #192

rohrlich opened this issue Mar 17, 2020 · 8 comments

Comments

@rohrlich
Copy link
Contributor

  • Open SymbolsView on a file with few symbols
  • Change the active view to a file with many symbols (enough to require scrolling)
  • Click SymbolsView Refresh button
  • The scrollbar doesn't appear
  • Collapsing and expanding any part of the tree will add the scroll bar

Adding NeedsFullReRender() to RefreshAction() does not help.

@rcoreilly
Copy link
Contributor

I'm fixing this now. Note that adding the View and TreeView pointers in the SymTree node is not a good idea:

// SymTree is the root of a tree representing symbols of a package or file
type SymTree struct {
	SymNode
	NodeType reflect.Type `view:"-" json:"-" qxml:"-" desc:"type of node to create -- defaults to giv.FileNode but can use custom node types"`
	View     *SymbolsView
	TreeView *SymbolTreeView
}

this is used for every single node in the tree (could be a large number) and it is only using the TreeView for the root node, unnecessarily, and it doesn't seem to use the View node it all. These pointers are very costly from a GC perspective, and should be avoided whenever possible!

Anyway, should be faster now..

@rcoreilly
Copy link
Contributor

also NodeType not being used either :)

@rcoreilly
Copy link
Contributor

and now I just saw SymNode -- I see that the above statements are wrong! You were patterning this off of the FileTree I think -- that is not actually the best example b/c it requires significant special root-node functionality which we put in the FileTree node but in general you want to avoid that complication if possible -- in this case it is unnec. anyway, no performance issues though!

@rcoreilly
Copy link
Contributor

also don't need SRoot on SymNode..

@rcoreilly
Copy link
Contributor

all working now. code much simpler :)

@rcoreilly
Copy link
Contributor

ps. scrollbar updating required: sfr.SetFullReRender() -- on the frame that contains the treeview. frames have the scrollbars based on their content sizes.. in principle this should be automatic -- will take another look at that..

@rcoreilly
Copy link
Contributor

yep it is necessary -- i guess the problem is that the tree view need for full rebuild is not propagating automatically up to parent frame -- in theory this kind of logic is in there but it isn't now -- will make a ticket.

@rohrlich
Copy link
Contributor Author

Thanks, for cleaning up this mess!

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

No branches or pull requests

2 participants