-
Notifications
You must be signed in to change notification settings - Fork 1.5k
UI: Add bookmark reordering #3757
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
base: v1.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2420,6 +2420,15 @@ If no bookmark exists, this function will do nothing. | |
| )"); | ||
| virtual void RemoveBookmark(uint32_t eventId) = 0; | ||
|
|
||
| DOCUMENT(R"(Reorders a bookmark by shifting its position in the bookmark list.This allows manual reordering of bookmarks. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like some spaces are missing: "list.This" -> "list. This" |
||
|
|
||
| The bookmark associated with the givenevent ID is swapped with the adjacent bookmark in the specified direction. | ||
|
|
||
| :param int EID: The event ID of the bookmark to move. | ||
| :param int direction: The direction to move. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this would benefit from being an enum instead of an int |
||
| )"); | ||
| virtual void ShiftBookmark(uint32_t EID, int32_t direction) = 0; | ||
|
|
||
| DOCUMENT(R"(Stores the dependent file data into the capture i.e. shader debug files. | ||
|
|
||
| This reads the contents of the dependent files and stores their file contents into the capture. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5344,9 +5344,7 @@ void EventBrowser::repopulateBookmarks() | |
|
|
||
| highlightBookmarks(); | ||
|
|
||
| m_BookmarkStripLayout->removeItem(m_BookmarkSpacer); | ||
| m_BookmarkStripLayout->addWidget(but); | ||
| m_BookmarkStripLayout->addItem(m_BookmarkSpacer); | ||
| m_BookmarkButtons[EID] = but; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this is required ? It is already set on line 5343 |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -5359,6 +5357,17 @@ void EventBrowser::repopulateBookmarks() | |
| m_BookmarkButtons.remove(EID); | ||
| } | ||
| } | ||
| m_BookmarkStripLayout->removeItem(m_BookmarkSpacer); | ||
| for(const EventBookmark &mark : bookmarks) | ||
| { | ||
| if(m_BookmarkButtons.contains(mark.eventId)) | ||
| { | ||
| QRClickToolButton *but = m_BookmarkButtons[mark.eventId]; | ||
| m_BookmarkStripLayout->removeWidget(but); | ||
| m_BookmarkStripLayout->addWidget(but); | ||
| } | ||
| } | ||
| m_BookmarkStripLayout->addItem(m_BookmarkSpacer); | ||
|
|
||
| ui->bookmarkStrip->setVisible(!bookmarks.isEmpty()); | ||
|
|
||
|
|
@@ -5393,12 +5402,17 @@ void EventBrowser::bookmarkContextMenu(QRClickToolButton *button, uint32_t EID) | |
|
|
||
| QAction renameBookmark(tr("&Rename"), this); | ||
| QAction deleteBookmark(tr("&Delete"), this); | ||
| QAction moveLeft(tr("Move &Left"), this); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be nice to add an appropriate icon for the new context actions. Perhaps |
||
| QAction moveRight(tr("Move &Right"), this); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. R : keyboard shortcut is already used by Rename : need to choose a different character |
||
|
|
||
| renameBookmark.setIcon(Icons::page_white_edit()); | ||
| deleteBookmark.setIcon(Icons::del()); | ||
|
|
||
| contextMenu.addAction(&renameBookmark); | ||
| contextMenu.addAction(&deleteBookmark); | ||
| contextMenu.addSeparator(); | ||
| contextMenu.addAction(&moveLeft); | ||
| contextMenu.addAction(&moveRight); | ||
|
|
||
| QObject::connect(&deleteBookmark, &QAction::triggered, [this, EID]() { | ||
| m_Ctx.RemoveBookmark(EID); | ||
|
|
@@ -5430,6 +5444,16 @@ void EventBrowser::bookmarkContextMenu(QRClickToolButton *button, uint32_t EID) | |
| } | ||
| }); | ||
|
|
||
| QObject::connect(&moveLeft, &QAction::triggered, [this, EID]() { | ||
| m_Ctx.ShiftBookmark(EID, -1); | ||
| repopulateBookmarks(); | ||
| }); | ||
|
|
||
| QObject::connect(&moveRight, &QAction::triggered, [this, EID]() { | ||
| m_Ctx.ShiftBookmark(EID, 1); | ||
| repopulateBookmarks(); | ||
| }); | ||
|
|
||
| RDDialog::show(&contextMenu, QCursor::pos()); | ||
| } | ||
|
|
||
|
|
||
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.
Be good to early return if "newIndex == index"