Add context menu to activity list #8585
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
TheOneRing
commented
Apr 28, 2021
TheOneRing
commented
TheOneRing
force-pushed
the
work/activity_context
branch
from
5cfe9a1
to
0d35ebf
Compare
|
Depends on #8584 |
TheOneRing
force-pushed
the
work/activity_context
branch
from
0d35ebf
to
a7825bb
Compare
|
Kudos, SonarCloud Quality Gate passed! |
Merged
fmoc
approved these changes
May 12, 2021
| case ActivityRole::Text: | ||
| return 120; | ||
| case ActivityRole::Account: | ||
| return 20; |
| @@ -174,55 +173,6 @@ void ActivityWidget::slotAccountActivityStatus(AccountState *ast, int statusCode | |||
| showLabels(); | |||
| } | |||
|
|
|||
| // FIXME: Reused from protocol widget. Move over to utilities. | |||
| QString ActivityWidget::timeString(QDateTime dt, QLocale::FormatType format) const | |||
There was a problem hiding this comment.
This change appears to be unrelated cleanup and should be in a separate commit. Otherwise, the Git history becomes quite useless.
There was a problem hiding this comment.
I just removed custom date handling, we just provide a qdate to the view and it takes care of the presentation.
As we no longer sort by the display string we don't need to have seconds in the display string.
| @@ -129,7 +131,6 @@ void ProtocolWidget::showContextMenu(QWidget *parent, ProtocolItemModel *model, | |||
|
|
|||
| void ProtocolWidget::slotItemContextMenu() | |||
| { | |||
| QModelIndexList list; | |||
There was a problem hiding this comment.
This seems like some unrelated cleanup as well.
| @@ -313,24 +253,23 @@ void ActivityWidget::slotBuildNotificationDisplay(const ActivityList &list) | |||
|
|
|||
| const auto accId = list.first().uuid(); | |||
| QList<Activity::Identifier> strayCats; | |||
| for (const auto &id : _widgetForNotifId.keys()) { | |||
| NotificationWidget *widget = _widgetForNotifId[id]; | |||
| for (auto it = _widgetForNotifId.cbegin(); it != _widgetForNotifId.cend(); ++it) { | |||
There was a problem hiding this comment.
Is this change somehow related to the topic of the PR? I don't see the relation between using an iterator here and the new context menu.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.