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

Improvements to metadata window and hiding open refine dialog #9435

Closed
wants to merge 8 commits into from

Conversation

Fidel365
Copy link
Contributor

Fixes issue partly issues #9374, #9368, #9383
Replaces pr #9390
@rdstern this is ready for review
@N-thony pr #9393 was dealing with resetting the window that I already did, unless it had other changes

Comment on lines 718 to 722
Private Sub ClearAllPanels()
splDataOutput.Panel1.Controls.Clear()
splMetadata.Panel1.Controls.Clear()
splExtraWindows.Panel2.Controls.Clear()
End Sub
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need this sub

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When swapping data and column metadata, then data with dataframe metadata or script, it seems to be stuck on the previous window, so, I need to clear it everytime I select a different option.

@N-thony
Copy link
Collaborator

N-thony commented Feb 11, 2025

@Fidel365, when I have all the windows visible, and click on Swap Data and Metadata I get below.
image
when I click on Reset, it goes back to normal but now when I click on Dataframe Metadata to display its window, it doesn't exist anymore.
image
image
similar with log script...
image

@rdstern
Copy link
Collaborator

rdstern commented Feb 11, 2025

@Fidel365 I notice in the pictures above, that the Data View is now there twice.
I think you (well all of us) ae paying a heavy price fore putting multiple issues together. I hope you and the code and maybe @N-thony can disentangle all this?

@Fidel365
Copy link
Contributor Author

@rdstern I had power issues throughout today and yesterday as fiber cables are being installed in our area, but I ussually have stable power at night, I'm going to tidy this up tonight and hopefully tomorrow I will be back working to.normal hours.

@Fidel365
Copy link
Contributor Author

@rdstern and @N-thony I made the changes requested.

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.

@Fidel365 nice and looks ok to me. It tidies the Prepare menu in a small way and enhances the toolbar.
@N-thony over to you. Delighted if it can be included in this release, but not essential.

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.

@Fidel365 please check my comments and be careful with the changes here, this is the main form.
@rdstern you need to check this more carefully as it is our main form.

@@ -1185,7 +1221,7 @@ Public Class frmMain
Dim strCurrentStatus As String

strCurrentStatus = tstatus.Text
If clsRLink.bInstatObjectExists AndAlso ucrDataViewer.HasDataChanged Then
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason for removing this code AndAlso ucrDataViewer.HasDataChanged?
This flag is used to trigger the autosaving when the data has changed. removing it means we are back to autosaving anytime even if the data hasn't changed.

Comment on lines +710 to +715
Private Sub ClearAllPanels()
' splDataOutput.Panel1.Controls.Clear()
splMetadata.Panel1.Controls.Clear()
splMetadata.Panel2.Controls.Clear()
splExtraWindows.Panel2.Controls.Clear()
End Sub
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have tried without this sub, and it works fine, I still believe you don't need it.

Comment on lines +1710 to +1712
'UpdateSwapDataAndScript()
'UpdateSwapDataAndMetadata()
'UpdateSwapDataFrameAndMetadata()
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove if not needed?

Comment on lines +1916 to +1918
'UpdateSwapDataAndScript()
'UpdateSwapDataAndMetadata()
'UpdateSwapDataFrameAndMetadata()
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

@rdstern
Copy link
Collaborator

rdstern commented Feb 15, 2025

@N-thony I assume this can be closed?

@Fidel365
Copy link
Contributor Author

Thanks to @N-thony for these changes in pr #9445 including the changes for the open refine dialog.

@Fidel365 Fidel365 closed this Feb 15, 2025
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.

3 participants