Rewrite the painting part of agandaitem
ClosedPublic

Authored by ognarb on Oct 3 2018, 10:19 AM.

Details

Summary

Remove gradient in favor of a solid color
Rewrite how the corner radius and border are painted.

Test Plan

Compile and run in the 18.08.1stable branch. Still need testing in master.

Diff Detail

Repository
R76 PIM: Event Views
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 3450
Build 3468: arc lint + arc unit
ognarb created this revision.Oct 3 2018, 10:19 AM
Restricted Application added a project: KDE PIM. · View Herald TranscriptOct 3 2018, 10:19 AM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
ognarb requested review of this revision.Oct 3 2018, 10:19 AM
ognarb edited the test plan for this revision. (Show Details)Oct 3 2018, 10:29 AM
ognarb added a project: KDE PIM: Junior Jobs.
lbeltrame added a subscriber: lbeltrame.

As this is a visual change, please add screenshots (one for before, one for the after).

As this is a visual change, please add screenshots (one for before, one for the after).

Sure

Here is the old version:


Here is the new version:

mlaurent requested changes to this revision.Oct 3 2018, 11:18 AM
mlaurent added inline comments.
src/agenda/agendaitem.cpp
1179

const each time that it's a constant variable please

1183

QBrush brushSolid(Qt::SolidPattern);

This revision now requires changes to proceed.Oct 3 2018, 11:18 AM

When it will be correct this change will go to master no 18.08 branch.
Thanks

ognarb updated this revision to Diff 42779.Oct 3 2018, 11:40 AM
ognarb marked 2 inline comments as done.

Add const

ognarb updated this revision to Diff 42780.Oct 3 2018, 11:42 AM

Correct diff

dvratil added inline comments.Oct 3 2018, 11:57 AM
src/agenda/agendaitem.cpp
1175

Could you try how it looks with RADIUS = 2? That might make it look close to what Breeze widgets look like (the rounderd corners are barely visible there).

1197

I believe you should get here if an event in agenda spans multiple days: then the top is rounded only on the first day and the bottom is rounded only on the end day.

Also, does this affect only the Agenda view (day/week view), or the month view as well? We should probably fix both, otherwise it will look weirdly inconsistent...

ognarb updated this revision to Diff 42786.Oct 3 2018, 12:23 PM
ognarb marked an inline comment as done.
  • Fix roundBottom and apply RADIUS=2

Also, does this affect only the Agenda view (day/week view), or the month view as well? We should probably fix both, otherwise it will look weirdly inconsistent...

It's only the agenda view, I wanted to do the month view in an other differential, but I can also work on it in this differential.

I also tried with RADIUS = 2 and it looks better

.

src/agenda/agendaitem.cpp
1197

Thanks I, found a bug

dvratil accepted this revision.Oct 3 2018, 12:42 PM

I'm fine with a separate diff for month view, I just didn't know from the top of my head which parts of the code are shared between those views.

From my side, this is good to land - but only in master please (it's not a bugfix, so it shouldn't go to stable branch).

ognarb marked 2 inline comments as done.Oct 3 2018, 1:07 PM

I'm fine with a separate diff for month view, I just didn't know from the top of my head which parts of the code are shared between those views.

From my side, this is good to land - but only in master please (it's not a bugfix, so it shouldn't go to stable branch).

I'm unable to build korganizer with master (to much dependencies to build). Do you know a way to build master in Arch Linux or in a VM? And an other question, when I am able to build master, can I push to master and how?

knauss added a subscriber: knauss.Oct 3 2018, 1:19 PM

I recommend to use https://community.kde.org/KDE_PIM/Docker to create a build environment for master.
Do you have already a identity acount (https://identity.kde.org/)? Otherwise others can push for you, if you tell us your mailaddress to add to the commits.
Normally we push the first couple of patches to get known each other and afterwards do the identity account, as you have full access to all KDE repos with an identity account.

No problem, the codebase is similar enough that if you code against the 18.08 branch the patches should apply on master without any big issues. If you don't have commit access (or even if you do and can't test against master), we can check and apply the patch for you :)

Unless someone else steps up, I'll do it tonight or tomorrow morning :)

ognarb added a comment.EditedOct 3 2018, 1:45 PM

I recommend to use https://community.kde.org/KDE_PIM/Docker to create a build environment for master.
Do you have already a identity acount (https://identity.kde.org/)? Otherwise others can push for you, if you tell us your mailaddress to add to the commits.
Normally we push the first couple of patches to get known each other and afterwards do the identity account, as you have full access to all KDE repos with an identity account.

Ok, it look like I have an identity account, but without developer privilege. My email address is schwancarl @ protonmail.com.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 6 2018, 12:05 PM
This revision was automatically updated to reflect the committed changes.