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

Improved Hide/Unhide Dataframe(s) dialog #7905

Merged
merged 13 commits into from Nov 25, 2022

Conversation

Vitalis95
Copy link
Contributor

Fixes #7533
@rdstern @N-thony @lloyddewit , I have added 2 radio buttons(Hide &Unhide), this improvements simplify the dialog for the user

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 I am not sure how this is supposed to work?
image

I had a number of data frames and hid some using the bottom right-click. (They just hid without showing the dialogue.) Then I pressed Unhide and it gave me your new dialogue. I then tried to unhide those that were hidden, though it listed all of them. And I couldn't use Add or double-click to put them into the receiver in unhide. They did go into the receiver with Hide. (But they were already hidden!

@Vitalis95
Copy link
Contributor Author

@rdstern , have a look at it.

@N-thony
Copy link
Collaborator

N-thony commented Oct 24, 2022

@derekagorhom can you peer review this?

derekagorhom
derekagorhom previously approved these changes Oct 24, 2022
Copy link
Contributor

@lloyddewit lloyddewit left a comment

Choose a reason for hiding this comment

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

looks good, just some small comments

instat/dlgHideDataframes.vb Outdated Show resolved Hide resolved
instat/dlgHideDataframes.vb Outdated Show resolved Hide resolved
instat/dlgHideDataframes.vb Outdated Show resolved Hide resolved
@N-thony
Copy link
Collaborator

N-thony commented Nov 3, 2022

@Vitalis95 is this ready for re-review?

@Vitalis95
Copy link
Contributor Author

@N-thony , yes. Could you test it?

@Vitalis95
Copy link
Contributor Author

@N-thony , the reset issue is now ok.

N-thony
N-thony previously approved these changes Nov 14, 2022
Copy link
Collaborator

@N-thony N-thony left a comment

Choose a reason for hiding this comment

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

@lloyddewit over to you

@rdstern
Copy link
Collaborator

rdstern commented Nov 18, 2022

@Vitalis95 this is looking pretty good now.
So Hide, clicking at the bottom just hides the current data frame, and Unhide takes you to the dialogue.
a) I was initially thinking that we should disable unhide, at the bottom, when there is nothing to hide. But I now suggest we leave it as it is, as it is a neat way of getting to the Hide/Unhide dialogue!
b) But it is odd that when you click on Unhide you get to Hide in the dialogue. Could you either swap the top radio button s round or go to the unhide (second one) initially. I quite like Hide before Unhide in the dialogue. So I'd prefer to go to the second - Unhide - when you first click on Unhide at the bottom. (Ideally you then have hide as the default if you first go to the dialogue itself.) So keep it as it is now, if that's confusing!
c) But one change you need is that the dialogue remembers the old setting - and this time it shouldn't. Because it is now unhidden! So clear the Unhide receiver when you unhide. And I assume you want to clear the Hide receiver also when you hide through the dialogue!!!

@Vitalis95
Copy link
Contributor Author

@rdstern , have a look at it for part c).

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 looks fine now. Thanks.
Once @lloyddewit approves it will be good to merge.

@lloyddewit lloyddewit changed the title Improving Hide/Unhide Dataframe(s) dialog Improved Hide/Unhide Dataframe(s) dialog Nov 25, 2022
@lloyddewit lloyddewit merged commit d6ac27a into IDEMSInternational:master Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhide Data Frame seems to not work
5 participants