3.0 regression: search field no longer stays visible

[Vadim Peretokin]

Search button no longer toggles search to show the results panel or not - instead it toggles search showing up itself or not. This is a regression since search didn't take up that much space before in the collapsed view and there's demand for the old behaviour (http://forums.achaea.com/discussion/comment/356337/#Comment_356337)

[Stephen Lyons (slysven)]

Humm, I remember this, commit-10649019416655df3e6499d83862b09e88f803ed {2016-02-18 19:34:01}

The QWidget then called "mpSearchArea" (but renamed there to widget_searchArea") was defined in the trigger_editor.ui file and was the container for all the search stuff, including the QComboBox that was the place to enter search terms, but in the dlgTriggerEditor constructor the value the pointer mpSearchArea pointed to was assigned a *DIFFERENT* widget namely the QTreeWidget tree_widget_search_results_main even though it seemed to be the "handle" for the whole of the search area. This didn't make sense to me.

I found the control labelled button_searchAreaClose and having a "close" style icon had it's clicked signal (it not being a "Checkable" button, clicked() would be sent on every activation) wired to a slot that toggled the state of the search area (as well as the big "Search" button) - for that to make sense it seemed to me that the "close" button would be expected to hide itself when clicked.

Together these gave me the impression that it was intended that clicking the "Search" button would show/hide the entire search area and that the "close" button would hide it. This was not happening as after the Search Area was brought up by the "Search" button the Search term entry box would not then go away!

Remember that the same "close" button icon also is used on the system message area on the top right of the editor and in that location it causes the system message area to hide itself completely.

It was my belief that I was fixing a bug when I recoded the behaviour to completely hide the search area.

Given that it is reported that some users want something other that want is currently the case it is feasible to add a second button perhaps called "button_searchAreaResultsHide" underneath the first which can clear/hide the *results* part of the search area i.e. that part with the identifier "treeWidget_searchResults" - I can visualise the sort of code to do this in an additional SLOT method to the dlgTrigggerEditor class.

[Vadim Peretokin (vperetokin)]

Okay, we misused an icon, but as far as the users are concerned, the functionality before worked OK, the functionality before does not work OK. Let's replace it with one of those expand/collapse icons then, but the feedback is clear, the old behaviour needs to come back. There's no real reason to use up more real estate either by adding an option to completely close it - completely closing it was not a feature that was requested.

[Vadim Peretokin (vperetokin)]

There were two ways to understand the situation - it was either a bug or a misused icon - in this case it was the latter, let's bring the behaviour back and change the button's icon.

[Stephen Lyons (slysven)]

I have a PR to push out against the release_30 branch that does this but am not yet sure whether that is the place to request a pull against...

[Vadim Peretokin (vperetokin)]

I'm not sure either. I was hoping to freeze release_30 so we can merge and
be done with it. However the need to do a forthnightly update where we fix
loading of 17, 18 map formats means that it won't really be frozen. I guess
let's put it in, at the risk of starting a slippery slope that we just
never get rid of release_30...

On Sun, Mar 26, 2017 at 2:05 PM Stephen Lyons <email address hidden>
wrote:

I have a PR to push out against the release_30 branch that does this but
am not yet sure whether that is the place to request a pull against...

--
You received this bug notification because you are subscribed to the bug
report.
https://bugs.launchpad.net/bugs/1675988

Title:
  3.0 regression: search field no longer stays visible

To manage notifications about this bug go to:
https://bugs.launchpad.net/mudlet/+bug/1675988/+subscriptions

[Vadim Peretokin (vperetokin)]

Yes, let's put it into release_30 and then update development_merge_30 with it too: #1 feedback in some discussions on 3.0 is this change in behaviour

Vadi - Yesterday at 4:02 PM
Just about a week after 3.0. How's everyone going with it? Upgraded / not yet / love it / hate it / same ?
Klendathu - Yesterday at 4:03 PM
Working without any problems on my desktop and windows 10 tablet. Only criticism at this point is that the search box in the script editor is hidden by default
MUHAHAHAHA! - Yesterday at 4:40 PM
yeah...I had to get help for that lol

[Vadim Peretokin (vperetokin)]

Migrating issues to Github, please follow the new discussion here: https://github.com/Mudlet/Mudlet/issues/508

This issue needs to be closed and there is no appropriate status, so will set it to "Opinion" just for migration purposes.