davidedmundson added a comment.

  > i don't think i like the idea of adding yet another setting on this already 
rather fragile code
  
  I think this explains why you haven't had a review, this existing code is 
already sketchy as-is :/
  But we should be trying to fix that not just all of us ignoring it. Sorry 
about that.
  
  So this clock has 4 states:
  
  1. horizontal
  2. horizontal small (date and tz appear alongside clock)
  3. vertical
  4. other ()
  
  Currently:
  1 & 2 choose the font size to fill the space vertically indefinitely
  3 chooses the font size that fits the space horizontally up to 3x normal font 
size
  4 is... frankly unreadable.
  
  With your patch:
  in states 1 and 2 you're explicitly setting the time zone label to a config 
value, and implicitly the date and TZ label to 0.7 of that.
  
    I could understand logic that limits the maximiumSize to a user defined 
value, but I don't want to be in a position where the user resizes a 
panel..then has to open all the config options of each widget and adjust things.
    Would keeping the fit mode but setting the maximumPixelSize to the config 
value work for you?
  
  States 3 and 4 are unchanged:
  
    I don't really want to have a config option that simply does nothing. Is 
there a reason it's not changed?

REVISION DETAIL
  https://phabricator.kde.org/D6764

To: januz, #plasma, #vdg
Cc: davidedmundson, ngraham, Zren, mart, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

Reply via email to