Skip to content

Copy from table / log level default / some settings tab improvements[CPP-281][CPP-403][CPP-483][CPP-474][CPP-476][CPP-460][CPP-461] #257

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Dec 11, 2021
Merged

Copy from table / log level default / some settings tab improvements[CPP-281][CPP-403][CPP-483][CPP-474][CPP-476][CPP-460][CPP-461] #257

merged 14 commits into from
Dec 11, 2021

Conversation

@john-michaelburke
Copy link
Collaborator

@john-michaelburke john-michaelburke commented Dec 1, 2021 •
edited by atlassian bot
Loading

Implements

  • Copying data from all tables (AFAICT) via OS native copy command.
  • Upon selecting or copying a row all other table rows deselect. (Not including observation tab tables https://swift-nav.atlassian.net/browse/CPP-517)
  • Changes default log level from INFO to WARNING to match old console.
  • Fixes column headers for Solution TableCPP-483
  • Allows user to select data from Settings tab textfield via mouseCPP-474
  • Fixes minimum widths and column headers for baseline table CPP-476
  • Implements slideview for settings tab.
  • Implements column resizing for settings table
  • Switches buttons in settings tab to icon over text and flat buttons.

Does not implement

  • There is poor anchoring for the settings pane which prevents text in the right pane from wrapping if the slide view is squeezed smaller and the text would otherwise need to wrap. It seemed a little too lengthy to include in this PR https://swift-nav.atlassian.net/browse/CPP-518

@john-michaelburke john-michaelburke requested review from a team and keithel-qt December 1, 2021 22:58
@john-michaelburke john-michaelburke force-pushed the john-michaelburke/copy-table branch from 4194ad0 to 923e9f0 Compare December 1, 2021 23:03
keithel-qt
keithel-qt suggested changes Dec 1, 2021
View reviewed changes
Copy link
Contributor

@keithel-qt keithel-qt left a comment

Choose a reason for hiding this comment

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

Left a few comments in here.
I really do think that making a SwiftTableView custom table view would make the table maintenance easier.

@notoriaga
Copy link
Contributor

@notoriaga can you think of a case where this could cause an issue for the settings tab? I had to swap out the variable with an alias to maintain your variable names referenced in the parent tab.

Hmm think it should be all good

keithel-qt and others added 9 commits December 1, 2021 17:29
@keithel-qt
Create SwiftTableView reusable component (#260)
959c995 
Create the SwiftTableView reusable component and use it in one location
in the app as a sample - the ThreadStateTable.
@john-michaelburke
Apply SwiftTableView all over.
b15bbd8
@keithel-qt
Prevent table rows from drawing over header
5153cc0 
Prevent the SolutionTable rows from drawing over the table header.
They were drawing outside of the boundary of their item. The solution is
to turn on clipping.

Clipping is not enabled by default because it does incur some
performance penalty, however in this case it is pretty negligible.

I also removed clipping for the items in the table header, as there was
no reason to clip those - they do not draw outside of their boundaries.
@keithel-qt
Remove Layout.preferredHeight from table header
8310525 
* Remove Layout.preferredHeight from Solution table's
  HorizontalHeaderView, as it already defines it's own height to be that
  value.
@keithel-qt
Use click instead of press for table row selection
1676a91 
* Select table rows on a mouse click instead of a mouse press. This is
  so that one can flick scroll a table view without selecting a row.
@john-michaelburke
Apply SwiftTableView all over.
5e058cd
@john-michaelburke
Clean up settings table.
a1cdbc8
@john-michaelburke
Merge branch 'main' of github.com:swift-nav/console_pp into john-mich…
f960a96 
…aelburke/copy-table
@john-michaelburke
Merge branch 'john-michaelburke/copy-table' of github.com:swift-nav/c…
bed44d7 
…onsole_pp into john-michaelburke/copy-table
@john-michaelburke
Copy link
Collaborator Author

@keithel-qt I implemented the table everywhere except for the Observation tab and additionally tried to sprucen up the Settings tab slightly. I switched the buttons to be flat and above the icons to match the old console. I also added a SplitView so we could resize the pane and added column resizing functionality.

I was wondering if you could do a pass over the settings tab for me. I'm struggling to get the anchoring between the table and SplitView working correctly. It seems after resizing the columns and then moving the SplitView, the scrollbar remains where the splitview was last moved to and subsequently borks the splitview. Here is a video describing the behavior I'm trying to fix.

I think the issues I'm having may be related to relying on widths of parents but had difficulty getting anchors.fill: parent to solve all my problems 🤦

Kazam_screencast_00011.mp4

@keithel-qt
Copy link
Contributor

You still need a review on this, right?

@silverjam
Copy link
Contributor

You still need a review on this, right?

Yes

@john-michaelburke
Copy link
Collaborator Author

@keithel-qt false alarm I believe I have figured it out. One line missing seems to do the trick. I'm going to polish it off and push a commit.

@john-michaelburke john-michaelburke changed the title Copy from table and log level default.[CPP-281][CPP-403] Copy from table and log level default.[CPP-281][CPP-403][CPP-483][CPP-474][CPP-476] Dec 10, 2021
@john-michaelburke john-michaelburke changed the title Copy from table and log level default.[CPP-281][CPP-403][CPP-483][CPP-474][CPP-476] Copy from table / log level default / some settings tab improvements[CPP-281][CPP-403][CPP-483][CPP-474][CPP-476] Dec 10, 2021
@john-michaelburke john-michaelburke force-pushed the john-michaelburke/copy-table branch from 929c098 to 6a6c575 Compare December 10, 2021 23:54
@john-michaelburke john-michaelburke merged commit 00b175c into main Dec 11, 2021
@john-michaelburke john-michaelburke deleted the john-michaelburke/copy-table branch December 11, 2021 00:08
@john-michaelburke john-michaelburke removed the request for review from a team December 11, 2021 00:09
@john-michaelburke john-michaelburke removed the request for review from keithel-qt December 11, 2021 00:09
@john-michaelburke john-michaelburke changed the title Copy from table / log level default / some settings tab improvements[CPP-281][CPP-403][CPP-483][CPP-474][CPP-476] Copy from table / log level default / some settings tab improvements[CPP-281][CPP-403][CPP-483][CPP-474][CPP-476][CPP-460][CPP-461] Dec 11, 2021
@keithel-qt
Copy link
Contributor

apologies - I was on vacation yesterday.

@john-michaelburke
Copy link
Collaborator Author

apologies - I was on vacation yesterday.

No problem I was able to figure out the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@keithel-qt keithel-qt keithel-qt requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@john-michaelburke @notoriaga @keithel-qt @silverjam