feat/issue-18186: Moving Activity Center to the main navigation bar + notification components updates
#18532
Conversation
Jenkins BuildsClick to see older builds (36)
|
This is a preparatory step for the upcoming refactor of `ActivityCenterPopup`, which will be transformed into a layout-based component instead of a popup. The directory has been relocated to `AppLayouts` to reflect its new architectural role. Note: This commit introduces no functional or visual changes.
Just added the option of customizing the `tooltip orientation`.
…w section layout - Added new `Activity Center` section item into the main navigation bar. - Switched `Activity Center` popup to layout. - Removed all usages of `StatusActivityCenterButton`, the related properties, signals and calls from all app layouts. - Updated section item notification count value so that the badge on the section item button is updated accordingly.
…dency Moved store dependency up to the layout level.
… remove `ActivityCenterStore` dependency and move store instantiation to root store - Removed the direct dependency on `ActivityCenterStore` from all Activity Center notification components, improving modularity and testability. - Refactored components to receive necessary data via props or higher-level containers. - Moved the instantiation of `ActivityCenterStore` to the main root store to centralize state management and ensure consistent access across the app. These changes contribute to a cleaner architecture and better separation of concerns.
… store directory - Moved `ActivityCenterStore` to a new root store directory, as it functions as a global, cross-domain store shared across the app.
…mprove `storybook` integration - Adjusted text sizes for better readability across devices. - Refined all notification delegate's layout for consistency. - Enhanced `Storybook` example with interactive controls for improved component testing.
noeliaSD
force-pushed
the
feat/issue-18186
branch
from
dd556fd to
ef62ffb
Compare
|
Pushed rebase! |
|
i am looking at autotests for this PR , will commit fixes when ready |
| id: root | ||
|
|
||
| spacing: 8 | ||
| width: parent.width |
There was a problem hiding this comment.
should be defined in the parent, only implicit widths here
Screen.Recording.2025-08-08.at.14.54.32.mov
3. Icon missing on this page but see it in the small view?
4. The alignment of contact is weird, if this is an existing issue or something you don't want to tackle right now, ignore it
|
mmmm, I'm on mac os, and you? Screen.Recording.2025-08-08.at.15.01.35.mov |
| @@ -0,0 +1,67 @@ | |||
| import QtQuick | |||
There was a problem hiding this comment.
this file and ui/app/AppLayouts/ActivityCenter/views/ActivityNotificationCommunityKicked.qml seem very similar visually, can we optimize to avoid code duplication?
There was a problem hiding this comment.
That's just a first step of refactoring. I just kept the notification components as they were, and just fixing some small visual issues. The plan is, when we have the complete redesign ready (milestone 2), start from scratch with the definition of an ActivityNotificationBase component and we will get rid of a lot of repeated code and duplications.
| import shared.controls | ||
| import utils | ||
|
|
||
| import "../controls" |
There was a problem hiding this comment.
no relative path imports ?
There was a problem hiding this comment.
Same comment here. I didn't touch the complete code.. I guess it's marked as new code bc of the dir changed, and file movements. We will address them in next steps.
There was a problem hiding this comment.
other places tooo.
its there in may files below
|
|
||
| property bool banned: true | ||
|
|
||
| required property var community |
There was a problem hiding this comment.
do we prefer passing this big community structure, or values within the structure
required property var communityName
required property var communityImage
required property var communityColor and so on...
There was a problem hiding this comment.
I'm thinking on what you suggest. I think it will be better for the component to be data agnostic. Indeed, probably it will be something like:
- titleText
- secondaryTitleText
- avatar
- contentAreaText
...
You could see the intention in the following component: AppLayouts/ActivityCenter/controls/NotificationBaseHeaderRow.qml. This is new, and better fits with the final shape expected
| QtObject { | ||
| id: d | ||
|
|
||
| property color stateTextColor: root.banned ? Theme.palette.dangerColor1 : |
|
|
||
| avatarComponent: StatusSmartIdenticon { | ||
| name: community ? community.name : "" | ||
| asset.color: community ? community.color : "black" |
There was a problem hiding this comment.
StatusColors.colors['black'] instead of black ?
| import StatusQ.Core.Theme | ||
| import StatusQ.Components | ||
|
|
||
| import shared |
There was a problem hiding this comment.
in all of these Notification Center files I think one imports are not needed like the shared ones
There was a problem hiding this comment.
Please check other too in all files
| import shared.views.chat | ||
|
|
||
| import "../controls" | ||
| import "../panels" |
There was a problem hiding this comment.
remove relative paths and check imports
| readonly property bool acceptedPending: membershipStatus === ActivityCenterTypes.ActivityCenterMembershipStatus.AcceptedPending | ||
| readonly property bool declinedPending: membershipStatus === ActivityCenterTypes.ActivityCenterMembershipStatus.DeclinedPending | ||
|
|
||
| property color stateColorText: { |
| @@ -0,0 +1,53 @@ | |||
| import QtQuick 2.15 | |||
There was a problem hiding this comment.
change to import QtQuick no versioning required
| emojiReactionsModel: appMain.rootStore.emojiReactionsModel | ||
| openCreateChat: createChatView.opened | ||
| walletStore: WalletStores.RootStore | ||
| isChatSectionModule: true |
There was a problem hiding this comment.
It's just moved exactly the same way the popup was created before. Anyway this property means that this specific store is created and related to a chat / 1:1 module and not a community one.
| import shared.panels | ||
| import utils | ||
|
|
||
| import "../controls" |
There was a problem hiding this comment.
avoid relative imports
| import shared.panels | ||
| import utils | ||
|
|
||
| import "../panels" |
There was a problem hiding this comment.
remove relative imports
There was a problem hiding this comment.
check if imports are really required, some may not be as I can see
| import shared.panels | ||
| import utils | ||
|
|
||
| import "../panels" |
There was a problem hiding this comment.
remove relative imports
| @@ -13,6 +13,12 @@ import "../controls" | |||
| ActivityNotificationMessage { | |||
| id: root | |||
|
|
|||
| required property var community | |||
There was a problem hiding this comment.
same here if we want to pass more granular properties instead of structs
There was a problem hiding this comment.
We will address these things in next refactoring steps. Not the scope for this pr
|
|
||
| StatusBaseText { | ||
| Layout.fillWidth: true | ||
| Layout.maximumHeight: 50 |
There was a problem hiding this comment.
why is this required ?
| import QtQuick | ||
| import QtQuick.Controls | ||
| import QtQuick.Layouts | ||
| import Qt5Compat.GraphicalEffects |
There was a problem hiding this comment.
unused import, checked other below too please.
| @@ -0,0 +1,795 @@ | |||
| import QtQuick | |||
There was a problem hiding this comment.
Just a question. Wouldn't we want to add a story book page for this as well ?
| background: Rectangle { | ||
| id: backgroundItem | ||
| radius: 6 | ||
| color: mouse.containsMouse ? Theme.palette.primaryColor3 : "transparent" |
There was a problem hiding this comment.
nit: Theme.Palette.transparent
me as well, just checkout master and its there |
| @@ -750,6 +750,25 @@ method load*[T]( | |||
| if activeSectionId == nodeManagementSectionItem.id: | |||
| activeSection = nodeManagementSectionItem | |||
| # Activity Center Section | |||
There was a problem hiding this comment.
This is for another (big) task. But I'm looking forward to have this model removed from nim side. This model mixing apples and oranges is a nightmare...
There was a problem hiding this comment.
Yeah, I agree. We can take a look at it in the next step (milestone 3 in the epic plan ;) )
| @@ -15,12 +15,13 @@ import "../controls" | |||
| ActivityNotificationBase { | |||
| id: root | |||
|
|
|||
| required property var community | |||
|
|
|||
| signal setActiveCommunity(string communityId) | |||
There was a problem hiding this comment.
please follow the convention that we already broadly use: setActiveCommunityRequested as this component by calling that doesn't set anything, it's just asking for that and needs to be handled externally.
Thanks for the review @Khushboo-dev-cpp ! I will only address the issues related to the new code but not the ones related to the existing code since this PR can grow too much and was not part of this scope. In favor, I'll create a follow-up task, that indeed, was already in my plan, were we will work on each particular notification component (together with the new designs) and make them pretty from outside but also clean and organized code ;) Hope it makes sense to you too! |
|
@noeliaSD @Khushboo-dev-cpp @caybro , there is no Community Portal to the left (pls check left panel). Are we removing it? (how it looks in master, pls check the Communities Portal button with orange dot) |
|
clicking accept contact request is not doing anything btw Screen.Recording.2025-08-10.at.16.30.45.mov |
anastasiyaig
force-pushed
the
feat/issue-18186
branch
from
7f36252 to
9645f30
Compare
anastasiyaig
force-pushed
the
feat/issue-18186
branch
from
9645f30 to
99f6881
Compare
Nope, not removing anything; both things look like legit bugs to me |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
While testing these changes, I became doubtful about this solution. It's all about ease of use, at least on desktop:
- most of the time the right part of the screen remains not used. When reviewing mentions we jump to conversation every time we click one. To continue with mentions we need to go back to notification section.
- currently reviewing process is additionally harder because we lose position on every click:
Screen.Recording.2025-08-26.at.11.37.38.mov
Other minor issues noticed:
- placeholder text could be centered
- hover is unpredictable, should work always when mouse is over:
Screencast.from.26.08.2025.11.24.15.webm
- accept/reject buttons doesn't work, only way to do that is via menu/popup
Closes #18186
What does the PR do
This PR introduces a refactor of the
Activity Center, transitioning it from a popup-based component to a layout-based component. In parallel, it includes improvements tonotification componentsand a small feature enhancement toStatusBetaTag.📝 NOTE for the reviewer
Key Changes
Refactor: Activity Center Architecture
Moved
ActivityCenterdirectory toAppLayoutsThis prepares for its transformation into a layout-based section, reflecting its new architectural role.
No functional or visual changes introduced at this stage.
Converted
Activity Centerfrom popup to layoutStatusActivityCenterButton, related signals, properties, and logic across layouts.Decoupled internal dependencies
Chat.RootStorefromActivityCenterNotifications.ActivityCenterStoredependency from individual notification components.Centralized state management
ActivityCenterStoreinstantiation to the root store.ActivityCenterStoreto a new root-level directory to reflect its global, cross-domain nature.UI & Storybook Improvements
Activity Center notificationdelegates.Storybook integrationwith new interactive controls for easier testing.Architecture Benefits
Affected areas
The complete
Activity Centerfeature and related flowsArchitecture compliance
My PR is consistent with this document: QML Architecture Guidelines
Screencapture of the functionality
Screen.Recording.2025-08-07.at.22.42.02.mov
Screen.Recording.2025-08-07.at.23.01.50.mov
Impact on end user
The location and user experience of the
Activity Center notificationswill change, as they are now integrated into the main navigation bar rather than displayed as a popup.How to test
Risk
There is a moderate risk of introducing regressions across the
Activity Center notification feature, particularly in terms of UI consistency and behavior.