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

Added File Menu element to viz.ui #1570

Merged
merged 1 commit into from Jun 23, 2018

Conversation

Projects
5 participants
@karandeepSJ
Copy link
Contributor

karandeepSJ commented Jun 17, 2018

Added a FileMenu2D that uses a ListBox2D to display all files and directories in the file system. Files are shown with Blue color and directories with Green. Clicking on a directory will take the user inside it.

filemenu

@MarcCote

This comment has been minimized.

Copy link
Contributor

MarcCote commented Jun 17, 2018

I think this code could be more concise if it was not reimplementing scrolling and selection logic as it is already managed by the ListBox component.

Here my version (still WIP) that could use as a start point: master...MarcCote:enh_ui_filedialog3

@MarcCote MarcCote self-requested a review Jun 17, 2018

@karandeepSJ

This comment has been minimized.

Copy link
Contributor

karandeepSJ commented Jun 17, 2018

It's not reimplementing them. There are additional tasks that need to be done on scrolling and clicks, so I added more callbacks. For scrolling, we need to update the colors of the text in the slots, so the scroll_callback is handling that.

@MarcCote

This comment has been minimized.

Copy link
Contributor

MarcCote commented Jun 17, 2018

I see. Maybe it would be better (more modular) to have that logic in ListBox no?

We could have a list of "representation" along the values for all items in the list. Then, whenever an item needs to be displayed, we update its slots according to its values (already doing this) and representation (need to be implemented). That way would offer a modular approach to use special representation for files/folders in the FileMenu.

@dmreagan dmreagan added this to PR needs a review in Viz Module Jun 18, 2018

@karandeepSJ

This comment has been minimized.

Copy link
Contributor

karandeepSJ commented Jun 21, 2018

What could be the possible uses for a ListBox that require different representations?

@MarcCote

This comment has been minimized.

Copy link
Contributor

MarcCote commented Jun 21, 2018

It could allow a user to define which colors should be used to highlight the different items in the listbox (like you are doing for FileMenu).

@karandeepSJ

This comment has been minimized.

Copy link
Contributor

karandeepSJ commented Jun 22, 2018

Should I keep representation as a list of colors?

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 23, 2018

Indeed, this fixes the error that was on master. The remaining error is due to #1565, so I will go ahead and merge this.

@arokem arokem merged commit c8d2f17 into nipy:master Jun 23, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

Viz Module automation moved this from PR needs a review to Done Jun 23, 2018

@karandeepSJ

This comment has been minimized.

Copy link
Contributor

karandeepSJ commented Jun 23, 2018

Wrong PR :P. The one you want is #1574

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 23, 2018

Yeah - sorry about that. See #1577 for the reversion of this.

I think the right thing to do know is to merge #1577 and then reopen this PR. Again - so sorry about my confusion. I had two windows open simultaneously, and then acted in the wrong one.

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Jul 24, 2018

Revert is done, can you reopen this PR @karandeepSJ?

@MarcCote MarcCote referenced this pull request Aug 15, 2018

Closed

viz.ui.FileMenu2D #1618

@skoudoro skoudoro referenced this pull request Sep 20, 2018

Open

viz.ui.FileMenu2D #16

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Merge pull request nipy#1570 from karandeepSJ/FileMenu
Added File Menu element to viz.ui
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment