Period analysis dialog issues with messages #383
Conversation
|
Hi @dbenn , I know you are busy with Auth0; please review this only when you have time after any urgent things. |
2) Harmonics are searched within top-hits only, which makes more sense (IMHO - Max) 3) HarmonicInfoDialog: a procedure that shows a selected harmonic was unreasonably complicated. Why should we seek for the y coordinate? Let's display the x-position only. 4) The domain type was ignored in the HarmonicInfoDialog, which made displaying the selected harmonic not work if x of PERIOD type. 5) An algorithm of the hair-cross positioning/showing was changed to make it more intuitive (IMHO - Max)
There was a problem hiding this comment.
Hi @mpyat2, everything you're doing looks great and as always, I appreciate it!
I'm just pausing a bit to think through the message notification changes because it's a fairly core part of the way things operate and it's been around a long time.
The use of the identifier (name) would presumably be optional, right?
The fundamental issue seems to relate to the unintended reception of messages. I'm just trying to think of any other way to achieve the same effect that for example, involves the observer not subscribing to particular messages or subscribing to something more specific than is being subscribed to currently.
I can't really flaw the approach you're taking. Just thinking it through Max! :)
I have one suggested change in MessageBase though (see comment in that class).
| @@ -22,6 +22,8 @@ | |||
| */ | |||
| public class MessageBase { | |||
|
|
|||
| private String name; | |||
There was a problem hiding this comment.
I'm wondering about the use of name here. The intent is something like "destination" or "recipient" identifier I think. Is that correct?
| } | ||
|
|
||
| if (chartID == null) { | ||
| MessageBox.showMessageDialog("Find Harmonic", "Please select a tab with a chart"); |
There was a problem hiding this comment.
I am wondering about this. Why are we not permitting the top-hits (or data) table to be selected? It seems quite intuitive to me for that to be permitted in addition to a plot tab, especially since we are now limiting ourselves to searching top-hits (which is fine) for harmonics.
| Mediator.getInstance().getHarmonicSearchNotifier().notifyListeners(msg); | ||
| } | ||
|
|
||
| private MultiEntryComponentDialog createParamDialog() { | ||
| toleranceField = new DoubleField("Relative Frequency Tolerance", 0.0, 1.0, currentTolerance); |
|
Hi @dbenn , Thank you for the suggestions! The use of the message identifier is entirely optional. It is something like a tag (label) attached to the message. Currently, it is used in the period analysis dialogs only to distinguish between dialog instances that may coexist if the user leaves the previous dialogs open. I practiced this, and as we can see, I'm not unique. Well, 'name' probably is not the best name for the field (ha-ha, the sentence sounds funny). Maybe 'tag' sounds better? By the way, to get rid of multiple showing the harmonic dialog, I added a secondary identifier (iDstring) to HarmonicSearchResultMessage. Concerning blocking the harmonics dialog on the tabs with tables: I will try to remove this blocking and implement top-hit/data point selection. |
This addresses issues #381, #382, and at least partially #61 (IllegalArgumentException while selecting harmonics in HarmonicInfoDialog).