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

improved colour scale for topic orgs map #1085

Merged
merged 3 commits into from Mar 28, 2020

Conversation

@Daniel-Mietchen
Copy link
Collaborator

Daniel-Mietchen commented Mar 24, 2020

Initial attempt to address #1084

@Daniel-Mietchen

This comment has been minimized.

Copy link
Collaborator Author

Daniel-Mietchen commented Mar 24, 2020

Looks better. Probably need to drop the "results" from "10 results".

Screen Shot 2020-03-24 at 01 11 03

Screen Shot 2020-03-24 at 01 11 55

@Daniel-Mietchen

This comment has been minimized.

Copy link
Collaborator Author

Daniel-Mietchen commented Mar 24, 2020

Would be great to arrange the colours such that the least occupied layer (i.e. highest numbers) are always displayed on top. May require changing the direction of the IF clause.

@Daniel-Mietchen

This comment has been minimized.

Copy link
Collaborator Author

Daniel-Mietchen commented Mar 27, 2020

I played around with color rearrangement, but it does not seem to be possible to specify the order in which the layers are being stacked for display.

@Daniel-Mietchen

This comment has been minimized.

Copy link
Collaborator Author

Daniel-Mietchen commented Mar 27, 2020

In terms of changing direction of the IF clause, that query is here.

@egonw egonw self-requested a review Mar 27, 2020
Copy link
Collaborator

egonw left a comment

The query works for me. In that sense the patch looks fine.

However, why is the number of dots less? Also, I would suggest to add a legend outside the <iframe> to explain the colors.

@Daniel-Mietchen

This comment has been minimized.

Copy link
Collaborator Author

Daniel-Mietchen commented Mar 27, 2020

Not sure what you mean by the fewer dots. If that refers to the two pics in #1085 (comment) , then they do not have to be compared against each other but against the two pics in #1084 (comment) , which represent two different use cases behind the two changes in the patch (removing the LIMIT, adding 1 as another step).

@Daniel-Mietchen

This comment has been minimized.

Copy link
Collaborator Author

Daniel-Mietchen commented Mar 27, 2020

Not sure about the explanation of the dot colour either, but I just added #1087 for that.

@egonw

This comment has been minimized.

Copy link
Collaborator

egonw commented Mar 28, 2020

Not sure what you mean by the fewer dots. If that refers to the two pics in #1085 (comment) , then they do not have to be compared against each other

Oh, sorry, I thought they were before after :)

@egonw

This comment has been minimized.

Copy link
Collaborator

egonw commented Mar 28, 2020

Not sure about the explanation of the dot colour either, but I just added #1087 for that.

Ah. awesome. @Daniel-Mietchen, you can actually push additional patches to existing pull requests. I will see if I can merge in #1087 into this branch.

@egonw egonw self-requested a review Mar 28, 2020
@egonw
egonw approved these changes Mar 28, 2020
Copy link
Collaborator

egonw left a comment

I merged in the patch from #1087

@Daniel-Mietchen

This comment has been minimized.

Copy link
Collaborator Author

Daniel-Mietchen commented Mar 28, 2020

I purposefully kept the explanation commit separate, since I am not sure that's what we need. But I take your merge as a yes and am merging all of it into master now.

@Daniel-Mietchen Daniel-Mietchen merged commit 63e1835 into master Mar 28, 2020
1 check passed
1 check passed
security/snyk (fnielsen) No manifest changes detected
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.