Copy labels: "Zoomed" instead of "Visible (matrix)"

Issue #426 resolved
Anastasia Baryshnikova created an issue

USE CASE: WHAT DO YOU WANT TO DO?

Make commands simple and consistent

STEPS TO REPRODUCE AN ISSUE (OR TRIGGER A NEW FEATURE)

Open any matrix, right click on row or column labels and choose "Copy labels".

CURRENT BEHAVIOR

One of the options is "Visible (matrix)". It is not clear what "matrix" means.

EXPECTED BEHAVIOR

I think it should just be named "Zoomed". This would also be consistent with the Export option which is called "Visible". However, naming it zoomed distinguishes copying the visible portion of the matrix from the visible labels (which can be smaller in whizzing label mode).

DEVELOPERS ONLY SECTION

SUGGESTED CHANGE (Pseudocode optional)

none

FILES AFFECTED (where the changes will be implemented) - developers only

unknown

LEVEL OF EFFORT - developers only

minor

COMMENTS

Comments (27)

  1. Christopher Keil repo owner

    @abarysh I am going to try to explain why I decided to put "visible (matrix)" in the context menu. let me know what you think:

    Basically, the distinction was made because of the whizzing labels. The whizzing labels change with cursor movement (as it is meant to be). When you right-click on the labels to copy them to the clipboard the visible labels consequently depend on the cursor location which may change with even the slightest movement - even after right-clicking occurred.

    I found this circumstance very inconvenient when defining a range of labels you want to copy. Adding a plain "visible" to the context menu might be misunderstood as currently visible labels which are not the same as the visible matrix elements if whizzing mode is active.

    So the sensible alternative was too copy the visible matrix elements, and hence the "(matrix)" addition. If a specific range of labels should be copied, there's no need to make them somehow visible by moving the cursor and wait until the whizzing labels adjust... you can simply select those labels and copy selected.

  2. Robert Leach

    I don't understand. I haven't seen this functionality in a PR. It's not in master. This feature should not even be in the jar you're testing or in the release, at least not until it has gone through code review. Are you testing a jar from an un merged branch in addition to alpha3?

  3. Robert Leach

    Never mind. Poor memory apparently. Been so focussed on other issues. I forgot this feature was implemented.

  4. Robert Leach

    Could mitigate the issue by either:

    1. Only showing "Visible (matrix)" when in whizzing label mode and show just "Visible" when in fitted label mode...
    2. Change "(matrix)" to something that flows as a single statement, like "Visible data" or "Visible columns" or "Visible tiles" or simply "Visible matrix" (without parens)
    3. Could add an additional option to actually copy "Visible labels" that sits below "Visible matrix"
    4. Simply do not provide the "Visible..." option when whizzing labels are on

    ...or some combination of the above options.

    I agree with Anastasia's intent to make it simple and easy to interpret, but I agree with Chris's concern about ambiguity when whizzing labels are in effect.

  5. Anastasia Baryshnikova reporter

    Yep, I totally understand. I guess it's difficult to clearly convey this distinction in the name of the option. How about this:

    1) We keep the current functionality as is, but change the name to: - All - Selected - Zoomed (or Zoomed In?)

    2) We add an issue for the future to actually copy the visible labels (the ones displayed in whizzing mode), so that the options become: - All - Selected - Zoomed - Visible

  6. Robert Leach

    I like that. Succinct. I'm jealous I didn't think of that first.

    A minor consideration: How do you think "Zoomed" will be interpreted if the matrix is not zoomed at all?

  7. Anastasia Baryshnikova reporter

    Can we disable the Zoomed option (i.e., gray it out) if the matrix is fully zoomed out? This can be added to alpha4.

  8. Robert Leach

    That would make sense. Similarly, "Selected" should be grayed out if nothing is selected. It currently is selectable and copies an empty string.

    One other thing I noted (perhaps I should create an issue for it):

    Copying column labels (I think) should copy labels in a manner that makes them newline-delimited. Currently, they are copied as tab-delimited. I understand why that was implemented that way and there is a context where I agree that is the way to do it, but in the context of copying, I think the user would want a newline-delimtied list as opposed to a tab-delimited list.

    I think that a tab-delimited list is something you would want in an "export labels" feature, so that they can easily be applied/added to a similar cdt file.

  9. Robert Leach

    BTW @TreeView3Dev, MapContainer has datamembers for firstVisibleLabel and lastVisibleLabel, which are indexes into the data, so this should be just as easy as the other options - HOWEVER, I don't think that those are updated when whizzing labels are not active. They might be, but I doubt it, so you would have to check whizzing mode and grab firstVisible/lastVisible instead when whizzing is off.

  10. Robert Leach

    Should there also be a selection for "this label" - to copy the clicked label? The menu item name would be debatable: "This", "Label", "1", "Single", "Clicked",...

  11. Anastasia Baryshnikova reporter

    Yes, good point. The choices should be (in order):

    • This
    • Selected
    • Zoomed
    • All

    But I think all these new features can be added to alpha4. Now, we should just rename "Visible (matrix)" to "Zoomed", if possible.

  12. Robert Leach

    Don't forget visible:

    • This
    • Selected (disabled when no selection)
    • Zoomed (disabled when zoomed out fully)
    • Visible (only different from zoomed when whizzing - so possible only provide when whizzing is on)
    • All

    We're agreed on when to implement.

  13. Christopher Keil repo owner

    Renamed Copy Labels option Visible (matrix)

    The old option 'Visible matrix' has been renamed to 'Zoomed' in preparation to implement new features for alpha4

    Resolves: #426

    See also: -

    → <<cset 1ffca8822bf9>>

  14. Robert Leach
    • changed status to open

    I don't believe that this issue is fully implemented, though the part that is I would agree is sufficient for alpha03.

  15. Christopher Keil repo owner

    Renamed Copy Labels option Visible (matrix)

    The old option 'Visible matrix' has been renamed to 'Zoomed' in preparation to implement new features for alpha4

    Resolves: #426

    See also: -

    → <<cset 1ffca8822bf9>>

  16. Robert Leach

    Are you putting the other elements of this issue into another bitbucket issue @TreeView3Dev ? Because this issue is not fully implemented.

  17. Robert Leach

    Nevermind. I'll just break the issue up. I suppose I should have broken it up instead of edit it to update it based on the discussion in the comments anyway.

  18. Robert Leach

    OK, I have broken this up into separate issues:

    1. Issue #434 - Add "Visible" option
    2. Issue #435 - Add "This" option
    3. Issue #436 - Disable irrelevant/redundant items
    4. Issue #437 - Submenu order

    I re-edited this issue to only describe what the existing PR resolves.

    I marked them all alpha04 and changed this issue back to alpha03.

  19. Log in to comment