Fixed Bug 396528
AcceptedPublic

Authored by anujgoyal on Oct 7 2018, 12:12 PM.

Details

Reviewers
scottpetrovic
rempt
Group Reviewers
Krita
Summary

fixed the bug 396528

  • Increased the size of curveWidget and now it's taking all space available by own its own.
  • also in some of the sensors there is some explanation provided along with values in Qlabel.so,made the label to appear in the the Tool-tip for better view and also underlined the labels who have tool-tip for everyone to know that there must be something out there like tool-tip.( as discussed on irc with scottyp as this will going to help him on his patch).
  • Updated some minor changes in the dialog like size of push buttons etc.

Before

After

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
anujgoyal requested review of this revision.Oct 7 2018, 12:12 PM
anujgoyal created this revision.

This looks good to me.

Committer note:
Before we commit this to master, we need to figure out how this will work with 2 other patches that have brush editor changes.

  1. https://phabricator.kde.org/D16009 (this patch with adding tooltips)
  2. https://phabricator.kde.org/D16008 (giant UI patch with resizing most brush editor UI files to shrink it by 1/2)
  3. https://phabricator.kde.org/D15903 (patch to add input fields to the curve so people can see and change where a node is at)

To be able to merge these, I think we will need to apply patch #2 first, then apply patch #1 and #2. The main merge conflicts with all of these will be in the UI files. I don't think there is going to be much conflict in the actual code files, so I think figuring out how to get the UI files merged is going to be the tricky part.

My thinking now is t

This revision is now accepted and ready to land.Oct 7 2018, 1:16 PM

I think for merging the ui file the only option is that anyone of us have to make UI file by taking into account of all the ui files

rempt added a comment.Oct 10 2018, 1:42 PM

I think the aspect ratio of the widget itself needs to remain square?

rempt added a comment.Oct 10 2018, 1:57 PM

I would prefer a clear icon to be shown that would expand to show the help text on clicking. Tooltips are not very discoverable, especially if people are using a stylus.

anujgoyal updated this revision to Diff 43314.Oct 10 2018, 2:49 PM

Now the aspect ratio of the curve widget is square. you can checkout.
and currently working on the adding icon .

anujgoyal updated this revision to Diff 43477.Oct 12 2018, 4:08 PM
anujgoyal updated this revision to Diff 43478.Oct 12 2018, 4:13 PM

the above are the changed and the icon is added to label with tooltip .

anujgoyal updated this revision to Diff 43479.Oct 12 2018, 4:15 PM

i think that if the user sees a different icon on only the 3 options than the user will definitely try to hover over it either intentionally or unintentionally .and will get to know that we get the explanation on hovering over.what do you think .So i,don't think there is any need of clickable icon as far as i think.please provide your feedbacks .

i think its accepted but not commited .if required any changes or if its not useful than please let me know

@rempt please rereview this or commit it.

Anush, could you give us a firstname, lastname and email adress that we can attach to the commit?

@woltherav

It's not Anush . Full name is Anuj Goyal and email id is :- anujg935@gmail.com