This revision was automatically updated to reflect the committed changes.
Closed by commit R120:bf6b8353a73f: Fix various bugs introduced with heading
(authored by niccolove).
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=82062=82065
niccolove updated this revision to Diff 82062.
niccolove added a comment.
Remove inHistory change
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=82060=82062
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove added a comment.
Sorry, I'll make a different patch for that
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D29234
To: niccolove, #plasma, broulik
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack,
jraleigh, zachus,
broulik added a comment.
> I added one last thing
Can we please not mix behavior changes into a patch that fixes bugs and
regressions.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D29234
To: niccolove, #plasma, broulik
Cc: ngraham, plasma-devel,
niccolove requested review of this revision.
niccolove marked 4 inline comments as done.
niccolove added a comment.
I added one last thing: I feel like it was not a good idea to show the
heading in history for notifications that are not in group. I added a inHistory
bool, false by default,
niccolove updated this revision to Diff 82060.
niccolove added a comment.
Address feedback and add inHistory bool
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81791=82060
BRANCH
fix_notifications (branched from master)
REVISION
broulik accepted this revision.
broulik added inline comments.
This revision is now accepted and ready to land.
INLINE COMMENTS
> NotificationItem.qml:118
> +Layout.preferredWidth: notificationHeading.implicitWidth
> +id: headingElement
> +
Put `id` at the top
>
niccolove updated this revision to Diff 81791.
niccolove added a comment.
Fix very long application names
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81750=81791
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove updated this revision to Diff 81750.
niccolove added a comment.
Fix very long notification titles
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81748=81750
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove updated this revision to Diff 81748.
niccolove added a comment.
Address feedback
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81733=81748
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
broulik added inline comments.
INLINE COMMENTS
> NotificationItem.qml:116
> +Layout.fillWidth: true
> +implicitHeight: !notificationItem.inGroup ?
> notificationHeading.implicitHeight : 0
> +
When `inGroup` the height will be zero so you effectively leak the item
contents
niccolove updated this revision to Diff 81733.
niccolove added a comment.
Move lineSvg to root element
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81732=81733
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove updated this revision to Diff 81732.
niccolove added a comment.
Fix visibility
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81730=81732
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove planned changes to this revision.
niccolove added a comment.
Ah, there's a mistake on visibility, just a sec
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D29234
To: niccolove, #plasma, broulik
Cc: ngraham, plasma-devel, Orage, LeGast00n,
niccolove added a comment.
Keyboard navigation is now working again; however, pressing tab to focus
buttons seem to no longer be possible after this patch; looking at the code,
I'm given the impression that it was true before porting notifications to page
as well. Is that correct, or should
niccolove updated this revision to Diff 81730.
niccolove added a comment.
Fix keyboard navigation
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81727=81730
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove added a comment.
I used the Svg instead of the plasmoidHeading to avoid using a Control.
User-wise, it works correctly, but can I get your opinion code-wise?
INLINE COMMENTS
> broulik wrote in FullRepresentation.qml:454
> Move the `Svg` somewhere outside the delegate, I don't want
niccolove updated this revision to Diff 81727.
niccolove added a comment.
Remove plasmoidHeading element as it's a control
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81389=81727
BRANCH
fix_notifications (branched from master)
broulik added inline comments.
INLINE COMMENTS
> niccolove wrote in NotificationItem.qml:125
> I originally did this but I had a problem. It displays and works just as
> good, but it will throw a "you are using anchors in layouts" error in the
> console. Even though I disable the anchors in
niccolove updated this revision to Diff 81389.
niccolove added a comment.
Fix close button not clickable
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81372=81389
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove added inline comments.
INLINE COMMENTS
> broulik wrote in NotificationItem.qml:125
> Please use one of the `State` below for `inGroup` state
I originally did this but I had a problem. It displays and works just as good,
but it will throw a "you are using anchors in layouts" error in
broulik requested changes to this revision.
broulik added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> FullRepresentation.qml:454
> elementId: "vertical-line"
> -svg:
niccolove updated this revision to Diff 81372.
niccolove added a comment.
Fix spacing
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81368=81372
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove planned changes to this revision.
niccolove added a comment.
Ahh, this adds top margin in history. Let me fix.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D29234
To: niccolove, #plasma, broulik
Cc: ngraham, plasma-devel, Orage, LeGast00n,
niccolove updated this revision to Diff 81368.
niccolove added a comment.
Missing space
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81367=81368
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
ngraham added inline comments.
INLINE COMMENTS
> NotificationItem.qml:441
> when: notificationItem.inGroup
> +
> PropertyChanges {
unrelated change
> NotificationItem.qml:140
> +
> +MouseArea{
> +anchors.fill: parent
missing space before the curly
niccolove updated this revision to Diff 81367.
niccolove added a comment.
unrelated pt3
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81366=81367
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove updated this revision to Diff 81366.
niccolove added a comment.
Unrelated pt2
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81365=81366
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove updated this revision to Diff 81365.
niccolove added a comment.
Remove unrelated
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D29234?vs=81364=81365
BRANCH
fix_notifications (branched from master)
REVISION DETAIL
niccolove created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
niccolove requested review of this revision.
REVISION SUMMARY
This fixes:
- Indented notifications line (it's back)
- Heading being non-clickable
- Buttons not being right-aligned
30 matches
Mail list logo
