Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Options for draw order of points (issue #2) & Bug fix (issue #5) #14

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

marohrdanz
Copy link
Collaborator

@marohrdanz marohrdanz commented Aug 12, 2020

The main branch of this project draws points from the NGCHM in one way: randomizes them with the same seed each time (same seed so that the plot looks the same each time; it an be a little jarring for the same data to look different each time a plot is created.)

This PR adds three additional options and uses a gear menu dropdown to allow the user to choose. The 4 options are:

  1. Randomize with same seed (so plot looks the same every time, the default)
  2. Randomize with a different seed (maybe this might help analyze data in some situations?)
  3. Original order (i.e. same order as points arrived from NGCHM)
  4. By groups (ordered largest to smallest, so that the smaller groups aren't obscured by the larger ones)

The above addresses issue #2

This PR also addresses issue #5

Added an option to the gear menu for user to choose which order
to draw points: random (same seed each time), random (different seed
each time), original (same order as NGCHM), or according to batch
size (largest batch drawn first, then second largest, etc).

Addresses #2
@bmbroom
Copy link
Member

bmbroom commented Aug 12, 2020

Just considering the UI:

#3: I agree with not calling this NG-CHM order so that the plugin can be used in other tools. However, in the context of NG-CHMs I imagine this could be misunderstood. For instance, a user who just made an NG-CHM might think it refers to the order in their data file. Unfortunately, I can't think of another word that doesn't have that or similar issues. ("Source order" would be just as bad, "Display order" might get confusing too.) Perhaps we could extend the protocol to include a substitution field, such as "{{TOOLNAME}}". I imagine you'd want to check for that in (nearly) every string value.

#4. Until I saw the explanation in the pull request, I was wondering how group order would be set and all the complications involved. I can imagine users be similarly confused. ("I set group order, but how do I pick what order?") Perhaps rename to either "Group size" or even "Decreasing group size".

Change the text of the dropdown labels for 'original' and 'batches'
ordering to try and make them more clear to users.
@marohrdanz
Copy link
Collaborator Author

marohrdanz commented Aug 12, 2020

Good points.

For #3: I think it works use 'NG-CHM Order' in the dropdown label because that is in an ngchm-specific file. Things are modularized here such that there is one main plotting file (canvasDraw.js), and an additional separate file for interfacing with each external data source (here the script that handles the interfacing with the NG-CHM is ngchm.js)

For #4: What about 'Group Size', with the details relegated to the help '?'?

So the dropdown (for d9dd36c) would look like:

updated-dropdown

There is an issue where if the user moves the mouse very quickly
off the canvas, the last highlighted point remains highlighted.
To prevent this, clearing the highlight canvas whenever the mouse
leaves (points selected will remain selected)

Addresses #5
@marohrdanz marohrdanz changed the title Options for draw order of points (issue #2) Options for draw order of points (issue #2) & Bug fix (issue #5) Aug 13, 2020
Making this change based on input from team; trying to make the
language clearer.
Made small changes to clarify help text based on group feedback.
Copy link
Member

@bmbroom bmbroom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new "NG-CHM Order" render order entry is the only place in the "ngchm.js"
file that's specific to NG-CHMs. I'd like to keep it non-specific, so perhaps we should rename
the file to something like "tool-interface.js".

Perhaps use "Same Order" in the menu entry.

The helpText could say "order as sent from the enclosing tool (e.g. NG-CHM)".

@jmelott
Copy link

jmelott commented Dec 11, 2020

How about "Tool Order"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants