Added a SearchColumnNames button to TableBrowser - #4180
Conversation
|
Does this mean that https://github.com/nalgeon/sqlean is no longer compatible with ARM, because as far as I know, so should my changes not affect this? |
It rather seems like upstream changes cause our patch to be not up-to-date anymore. I guess @lucydodo needs to take a look at this, maybe an upstream patch is feasible to reduce the risk of breaking in the future. |
0ba7202 to
d31c432
Compare
|
@FriedrichFroebel Would you still like to proceed with the review of this PR? |
|
@lucydodo I Just pointed out the issue with our CI. For the remaining parts, I will leave this to persons with more knowledge, as my knowledge of C++ and Qt is rather limited ultimately. |
|
@FriedrichFroebel No worries, I'll take a look over the weekend. :) |
|
In the meantime, I'll try using GitHub Copilot to do a first-pass review. |
There was a problem hiding this comment.
🟡 Changes recommended
Shared search state, hidden-column matches, incorrect scrolling, and reversed navigation icons need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds column-name search to Browse Data.
Changes:
- Adds column-search controls and toolbar action.
- Implements case-sensitive navigation and column highlighting.
- Adds and registers a dedicated icon.
File summaries
| File | Description |
|---|---|
src/TableBrowser.ui |
Adds search UI and action. |
src/TableBrowser.h |
Declares column search logic. |
src/TableBrowser.cpp |
Implements search behavior. |
src/icons/page_find_column.svg |
Provides the new icon. |
src/icons/icons.qrc |
Registers the icon resource. |
Review details
- Files reviewed: 4/5 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (columnCount <= 0) | ||
| return; | ||
|
|
||
| static int lastFoundIndex = -1; |
| const QString columnName = m_model->headerData(i, Qt::Horizontal, Qt::EditRole).toString(); | ||
| if (!columnName.contains(searchText, caseSensitivity)) | ||
| return false; |
| ui->dataTable->selectionModel()->select(QItemSelection(top, bottom), | ||
| QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Columns); | ||
|
|
||
| ui->dataTable->horizontalScrollBar()->setValue(ui->dataTable->horizontalHeader()->sectionViewportPosition(i)); |
| </property> | ||
| <property name="icon"> | ||
| <iconset resource="icons/icons.qrc"> | ||
| <normaloff>:/icons/up</normaloff>:/icons/up</iconset> |
| </property> | ||
| <property name="icon"> | ||
| <iconset resource="icons/icons.qrc"> | ||
| <normaloff>:/icons/down</normaloff>:/icons/down</iconset> |
lucydodo
left a comment
lucydodo
left a comment
There was a problem hiding this comment.
LGTM, Now it's merged. Thank you for the contribution. :)
This pull request is in regards to: #3542.
Added a SearchColumnNames button to the TableBrowser. The button is placed next to the FindReplaceButton.
The button icon was created by using Inkscape to modify the page_find.svg, to ensure it would be of the same style as the other buttons.
When the SearchColumnNames button is pressed so will it open a dialog box that is almost the same as the FindDialogBox, so it would not make the UI inconsistent (and its simpler to copy code and just change its text and refrences)
The search functionality is modeled after how the FindSearch funktions and I think its a 1 to 1 in how it behaves, except that I made it highlight the full column instead of a individual cell.