feat(Adaptive navigation) Primary nav sidebar POC #19607
Conversation
Jenkins BuildsClick to see older builds (80)
|
caybro
force-pushed
the
19593-adaptive-navigation-primary-nav-sidebar-poc
branch
from
6f812f2 to
d0b8239
Compare
micieslak
left a comment
micieslak
left a comment
There was a problem hiding this comment.
Nice work, looks promising!
Some suggestions regarding code structure in comments.
I played a bit with Theme, and it seems that some adjustments are needed:
Screencast.from.18.12.2025.12.25.18.webm
IMO, there is some space for visual improvements regarding boundaries of the scrollables. Maybe we could apply some shadow there or at least hide behind the separator, not earlier (for the bottom one). On top maybe some separator would help as well.
| ProfileButton { | ||
| objectName: "statusProfileNavBarTabButton" | ||
| Layout.alignment: Qt.AlignHCenter | ||
| name: root.profileStore.name |
There was a problem hiding this comment.
I'd love to find a good way to separate the nav bar component from the dependency of all that stuff needed by profile menu. It may require refactoring ProfileButton as well. That button could just emit request to open menu, probably passing location as an argument.
There was a problem hiding this comment.
Yeah... I was thinking the same but didn't want to do it here as part of the POC; file a separate issue?
| readonly property int containerBgRadius: Theme.padding // 16 | ||
|
|
||
| // models | ||
| readonly property var sectionsModelInternal: SortFilterProxyModel { |
There was a problem hiding this comment.
Following the rule that UI components just display data limiting transformation to minimum I think we should externalize those transformation. That component displays 3 models and therefore it could just take tree models from outside.
There was a problem hiding this comment.
Other detail - instead of creating sectionsModelInternal property in d we could define that SFPM at top level directly and set id sectionsModelInternal. It's still private with no access from outside and reduces indentation.
There was a problem hiding this comment.
Yeah... the plan was to exclude the data transformation to an adaptor at a later stage
Yup, the designs are still not final; I asked that same question regarding the overall scroll situation: 
|
Haha yeah, I tied the icon width/height to |
caybro
force-pushed
the
19593-adaptive-navigation-primary-nav-sidebar-poc
branch
2 times, most recently
from
87dff99 to
307d9a8
Compare
caybro
force-pushed
the
19593-adaptive-navigation-primary-nav-sidebar-poc
branch
4 times, most recently
from
6a83c6b to
45e8f8b
Compare
jrainville
left a comment
jrainville
left a comment
There was a problem hiding this comment.
LGTM, but I'll let actual UI experts approve 😄
|
Nav bar looks very good in the videos though 💯 |
- introduce `PrimaryNavSidebar{Button}` components
- update the main sections SVG icons
- added a basic QML test suite
- add a SB page for the sidebar POC
Fixes #19593
caybro
force-pushed
the
19593-adaptive-navigation-primary-nav-sidebar-poc
branch
from
45e8f8b to
c62972c
Compare
4 participants
What does the PR do
PrimaryNavSidebar{Button}componentsFixes #19593
Affected areas
Storybook only for now (AppMain later)
Architecture compliance
My PR is consistent with this document: QML Architecture Guidelines
Screencapture of the functionality
primary-nav-sidebar-demo.mp4
Impact on end user
Better AppMain navigation UX
How to test
Risk