-
Notifications
You must be signed in to change notification settings - Fork 105
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
Improving the view menu and hiding the open refine dialog #9390
Conversation
@Fidel365 that is coming on, but it also makes me nervous. So I have one more small task on #9366, in b) below. Then I suggest we merge. Then have a separate pull request on #9368. |
@rdstern I'm working to resolve the issue of the toolbar column metadata not coming at first load hence getting an empty column metadata, alongside the translation of the items in a fresh PR that will mainly tackle the other issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fidel365 I missed this and was waiting for it.
@N-thony I am approving in the hope it can get into this release. Be very nice, because visible, and completes the toolbar changes.
There is a small point not working yet, which should become a new minor issue. When the data and metadata are swapped, there is no tick in the toolbar drop-downs. But happy to live with that for now. The change is important to have soon.
@rdstern I think someone else changes on this may have been merged to.main, this was all well, and the checkboxes were working corresponding to each other, I see now even column metadata and dataframe metadata; not as before but now they have the same issue. |
@rdstern I can work on the last part you mentioned in a minute once this is merged, I'm unable to work here directly since I pulled the changes and It happen to have conflicts in vb. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@N-thony over to you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Private Sub ClearAllPanels() | ||
splDataOutput.Panel1.Controls.Clear() | ||
splMetadata.Panel1.Controls.Clear() | ||
splExtraWindows.Panel2.Controls.Clear() | ||
End Sub |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this needed?
Private Sub UpdateMenuStates(selectedMenuItem As ToolStripMenuItem) | ||
mnuViewSwapDataAndMetadata.Checked = (selectedMenuItem Is mnuViewSwapDataAndMetadata) | ||
mnuViewSwapDataAndDataframeMetadata.Checked = (selectedMenuItem Is mnuViewSwapDataAndDataframeMetadata) | ||
mnuViewSwapDataAndScript.Checked = (selectedMenuItem Is mnuViewSwapDataAndScript) | ||
End Sub |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also this, do we need it?
Private Sub mnuViewSwapDataAndDataframeMetadata_CheckStateChanged(sender As Object, e As EventArgs) Handles mnuViewSwapDataAndDataframeMetadata.CheckStateChanged | ||
If Not mnuViewSwapDataAndDataframeMetadata.Checked AndAlso Not mnuViewDataView.Checked AndAlso mnuViewDataFrameMetadata.Checked Then | ||
mnuViewDataFrameMetadata.Checked = False | ||
mnuViewDataView.Checked = True | ||
End If | ||
End Sub |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this sub?
@derekagorhom I can work on the changes all together in a pr fresh from this one, as I cannot push again here, I mean this can be merged and I pull request again for the changes |
Can you resolve my comments? |
splDataOutput.Panel1.Controls.Add(ucrColumnMeta) | ||
splMetadata.Panel1.Controls.Add(ucrDataViewer) | ||
mnuViewColumnMetadata.Text = "Data View" | ||
mnuViewDataView.Text = "Column Metadata" | ||
mnuSwapDataMetadata.Checked = True | ||
UpdateMenuStates(mnuViewSwapDataAndMetadata) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can simply have mnuViewSwapDataAndMetadata.Checked = True
here
splMetadata.Panel1.Controls.Add(ucrDataViewer) | ||
mnuViewColumnMetadata.Text = "Data View" | ||
mnuViewDataView.Text = "Dataframe Metadata" | ||
UpdateMenuStates(mnuViewSwapDataAndDataframeMetadata) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here mnuViewSwapDataAndDataframeMetadata.Checked = True
splDataOutput.Panel1.Controls.Add(ucrScriptWindow) | ||
splExtraWindows.Panel2.Controls.Add(ucrDataViewer) | ||
mnuViewLogScript.Text = "Data View" | ||
mnuViewDataView.Text = "Log/Script" | ||
mnuSwapDataLogScript.Checked = True | ||
UpdateMenuStates(mnuViewSwapDataAndScript) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mnuViewSwapDataAndScript.Checked = True
Fixes issues partially #9374, #9368 and #9366 and replaces PR #9383 .
@rdstern this is where I got, you can review it but I will finish the small remaining items on monday, Enjoy your weekend!