if the user did set an implicit size, keep it
ClosedPublic

Authored by mart on Mar 29 2017, 3:03 PM.

Details

Summary

if the user code did something like
IconItem {

implicitWidth: 32
implicitHeight: 32

}

then never automatically update implicitWidth or height.
this favors compatibility and fixes the desktop
toolbox appearance

Test Plan

textbox works, other icon items in plasmashell seems properly sized.
applet alternatives fix is still needed

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.Mar 29 2017, 3:03 PM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptMar 29 2017, 3:03 PM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
davidedmundson added inline comments.
src/declarativeimports/core/iconitem.cpp
92

this line seems leftover from something?

mart updated this revision to Diff 12975.Mar 29 2017, 3:09 PM
  • remove leftover
mart updated this revision to Diff 12977.Mar 29 2017, 3:14 PM
mart marked an inline comment as done.
  • use q_revision
mart updated this revision to Diff 12978.Mar 29 2017, 3:17 PM
  • revisions not useful here
hein added a subscriber: hein.Apr 3 2017, 1:14 PM

review talk

‎[22:08] ‎<‎Sho_‎>‎ notmart: if you redefine a property with different NOTIFY signals, do the other signals from the baseclass still work, too?
‎[22:09] ‎<‎Sho_‎>‎ notmart: also, why do you need different NOTIFY signals, can't you just emit the base class ones?
‎[22:09] ‎<‎notmart‎>‎ Sho_: the implicitWidthChanged signal seen from qml side should become the new one if i'm right
‎[22:09] --> s8321414 (~s8321414@150.117.137.90) has joined this channel.
‎[22:10] ‎<‎notmart‎>‎ no, apparently i can't put a signal from the superclass in the q_property definition :/
‎[22:10] ‎<‎Sho_‎>‎ hmm
‎[22:10] ‎<‎Sho_‎>‎ so if the base class emits implicitWidthChanged, the prop still notifies, too?
‎[22:10] ‎<‎Sho_‎>‎ or only on your implicitWidthChanged2?
‎[22:10] ‎<‎Sho_‎>‎ because if it's the latter you need more connects
‎[22:13] ‎<‎notmart‎>‎ hmm, i should test it a bit more i guess :/
‎[22:13] ‎<‎Sho_‎>‎ unit tests
‎[22:13] ‎<‎Sho_‎>‎ :-)
‎[22:13] ‎<‎notmart‎>‎ i don't like that patch as redefining properties in c++ is a really messy thing
‎[22:14] ‎<‎Sho_‎>‎ aye

mart updated this revision to Diff 13064.Apr 3 2017, 1:43 PM
  • always emit implicitWidth/HeightChanged2 when implicit changes
hein accepted this revision.Apr 3 2017, 1:49 PM
This revision is now accepted and ready to land.Apr 3 2017, 1:49 PM
This revision was automatically updated to reflect the committed changes.