D12944: [weather applet] Show a note why "Show temperature" is disabled

[Friedrich W. H. Kossebau]

kossebau abandoned this revision.
kossebau added a comment.


  No time to care for this more, someone else needs to take over if 
needed/wanted,

REPOSITORY
  R114 Plasma Addons

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

To: kossebau, #plasma
Cc: mart, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra

D12944: [weather applet] Show a note why "Show temperature" is disabled

[Nathaniel Graham]

ngraham added a comment.


  +1 for redoing the checkboxes to follow the HIG! There seems to be a bit too 
much padding between them, though.
  
  As for the message, how about "Temperature cannot be shown because this 
widget is a part of the system tray, so there would not be enough room."
  
  However, a nicer UI might be a way to actually show the temperature when it's 
a tray widget instead of the current image. Something like this:
  
Display today's: (o) Weather condition image
 ( ) High temperature
 ( ) Low temperature
 ( ) Current temperature
  
  That way there would actually be some options for people using the tray 
widget, instead of a disabled option sitting there taunting them that we have 
to explain or else we get bug reports.

REPOSITORY
  R114 Plasma Addons

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

To: kossebau, #plasma
Cc: mart, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12944: [weather applet] Show a note why "Show temperature" is disabled

[Friedrich W . H . Kossebau]

kossebau added a comment.


  Looking like this when triggered:
  F5854760: Screenshot_20180518_045938.png 

  
  Message text might need more improvement, proposals welcome. And fine to do 
master-only, just completing things now this has been started to deal with.

REPOSITORY
  R114 Plasma Addons

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

To: kossebau, #plasma
Cc: mart, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12944: [weather applet] Show a note why "Show temperature" is disabled

[Friedrich W . H . Kossebau]

kossebau updated this revision to Diff 34405.
kossebau added a comment.


  Using Kirigami.InlineMessage with layout now in control and improved message
  text (and updated to more HIG-conformant UI)

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12944?vs=34362&id=34405

BRANCH
  addnotewhyshowtempdisabled

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

AFFECTED FILES
  applets/weather/package/contents/ui/config/ConfigAppearance.qml

To: kossebau, #plasma
Cc: mart, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12944: [weather applet] Show a note why "Show temperature" is disabled

[Nathaniel Graham]

ngraham added a comment.


  No problem, take your time!

REPOSITORY
  R114 Plasma Addons

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

To: kossebau, #plasma
Cc: mart, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12944: [weather applet] Show a note why "Show temperature" is disabled

[Friedrich W . H . Kossebau]

kossebau added a comment.


  Meh, forgot about ensuring proper wordwrapping, but somehow
  
Layout.fillWidth: true
wrapMode: Text.WordWrap
  
  don't have the desired effect, seems the groupbox is messing into that.
  Screws this approach.
  
  In D12944#264142 , @mart wrote:
  
  > can you try how it looks with the component InlineMessage from Kirigami?
  
  
  Somehow InlineMessage does not play well with GridLayout :/ And then same 
wordwrap issue. Meh config pages and minimum width behaviour...
  
  In D12944#264144 , @ngraham wrote:
  
  > In what circumstance is it enabled? If it's only enabled when the weather 
forecast is disabled, perhaps we should instead use ratio buttons, like so:
  >
  >   Compact mode: (0) Show weather forecast
  > ( ) show temperature
  >  
  >
  
  
  What the info was trying to tell: showing in compact mode additionally the 
temperature is only available if used in a location where there are no size 
constraints.
  
  Seems this is not that quick & easy to solve, and given I have to run now, no 
chance to do this last minute dirty improvement for 5.13, People will survice 
this :)
  Will pick up later once I have landed some more changes which then again 
would influence this config page (which is the reason why I had forgotten about 
that issue, hope was to get those features done for 5.13).
  
  Thanks for quick reaction for now.

REPOSITORY
  R114 Plasma Addons

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

To: kossebau, #plasma
Cc: mart, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12944: [weather applet] Show a note why "Show temperature" is disabled

[Nathaniel Graham]

ngraham added a comment.


  In what circumstance is it enabled? If it's only enabled when the weather 
forecast is disabled, perhaps we should instead use ratio buttons, like so:
  
Compact mode: (0) Show weather forecast
  ( ) show temperature
  
  Not related to this patch of course, but your screenshot reminds me: we 
should move the labels to the right of the checkboxes to be in line with the 
KDE HIG: https://hig.kde.org/components/checkbox.html

REPOSITORY
  R114 Plasma Addons

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

To: kossebau, #plasma
Cc: mart, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12944: [weather applet] Show a note why "Show temperature" is disabled

[Marco Martin]

mart added a comment.


  can you try how it looks with the component InlineMessage from Kirigami?

REPOSITORY
  R114 Plasma Addons

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

To: kossebau, #plasma
Cc: mart, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D12944: [weather applet] Show a note why "Show temperature" is disabled

[Friedrich W . H . Kossebau]

kossebau added a comment.


  Looks like this when hit:
  F5853714: Screenshot_20180517_160131.png 


REPOSITORY
  R114 Plasma Addons

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

To: kossebau, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12944: [weather applet] Show a note why "Show temperature" is disabled

[Friedrich W . H . Kossebau]

kossebau created this revision.
kossebau added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  Giving the user a clue why the setting is disabled helps them understand
  things as intended instead of thinking of a bug.
  
  BUG: 389547

REPOSITORY
  R114 Plasma Addons

BRANCH
  master

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

AFFECTED FILES
  applets/weather/package/contents/ui/config/ConfigAppearance.qml

To: kossebau, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart