D10660: [Weather applet] Let Return invoke search

[Friedrich W . H . Kossebau]

kossebau added a comment.


  I would like to consider discarding this patch and the related introduction 
of the "acceptOnReturnPressed" property to Plasma config page concept only 
needed for this so far.
  
  I am currently drafting to rework the UI of the weather station page in the 
dialog, to add support for multiple stations (following request to match 
feature of the old yawp widget).
  
  And instead of having the current embedded weather station picker, whose 
interaction pattern has been reported to be confusing at least to some, go to 
follow the pattern as we know it from e.g. the file picker, where another 
separate subdialog is opened for just that purpose of selecting an item.

REPOSITORY
  R114 Plasma Addons

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

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

D10660: [Weather applet] Let Return invoke search

[Friedrich W . H . Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> ConfigWeatherStation.qml:33
> +
> +property bool acceptOnReturnPressed: !searchStringEdit.activeFocus || 
> searchStringEdit.length === 0
>  

could be "readonly property" as well, no?

REPOSITORY
  R114 Plasma Addons

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

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

D10660: [Weather applet] Let Return invoke search

[Marco Martin]

mart added a comment.


  yeah, for me that other patch is ok, is not optimal but should all be 
documented in line in that file
  
  maaaybe introducing a root component for those config uis.. not sure

REPOSITORY
  R114 Plasma Addons

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

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

D10660: [Weather applet] Let Return invoke search

[Friedrich W . H . Kossebau]

kossebau added a comment.


  In D10660#232255 , @mart wrote:
  
  > any news on this?
  
  
  Blocked a little by me having the desire to have the secret API used here 
made public documented, cmp. question to you in 
https://phabricator.kde.org/D10659#212230
  
  No alternative solution from myself otherwise, so if you think this is a 
future-proof solution, fine with me.

REPOSITORY
  R114 Plasma Addons

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

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

D10660: [Weather applet] Let Return invoke search

[Marco Martin]

mart added a comment.


  any news on this?

REPOSITORY
  R114 Plasma Addons

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

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

D10660: [Weather applet] Let Return invoke search

[Kai Uwe Broulik]

broulik edited the summary of this revision.

REPOSITORY
  R114 Plasma Addons

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

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

D10660: [Weather applet] Let Return invoke search

[Friedrich W . H . Kossebau]

kossebau added a comment.


  Not tested, but seems good to me by pure reading.

INLINE COMMENTS

> ConfigWeatherStation.qml:33
> +
> +property bool acceptOnReturnPressed: !searchStringEdit.activeFocus || 
> searchStringEdit.length === 0
>  

Slightly undecided: given above we use the `length` property as implicit bool, 
dping a value comparison here instead seems inconsistent.

Just saying, no own preference and not enough years of JS/QML done to have 
strong opinions about it :) Do we have some accepted JS tricks with Plasma code 
style?

REPOSITORY
  R114 Plasma Addons

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

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

D10660: [Weather applet] Let Return invoke search

[Kai Uwe Broulik]

broulik edited the summary of this revision.

REPOSITORY
  R114 Plasma Addons

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

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

D10660: [Weather applet] Let Return invoke search

[Kai Uwe Broulik]

broulik edited the summary of this revision.

REPOSITORY
  R114 Plasma Addons

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

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

D10660: [Weather applet] Let Return invoke search

[Kai Uwe Broulik]

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

REVISION SUMMARY
  Depends on D10659 

TEST PLAN
  Hit Return after opening dialog, closed
  Hit Return with search focused but nothing in it, closed
  Hit Return with search query, started search (no search providers seem to be 
enabled by default, though, and no hint is provided why I cannot search..)

REPOSITORY
  R114 Plasma Addons

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

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

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