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

Add Pixelmania version of RGB Mixer interactive #1305

Merged
merged 8 commits into from Aug 1, 2020

Conversation

courtneycb
Copy link
Contributor

@courtneycb courtneycb commented Jul 30, 2020

Changes

  • Add optional URL parameter mode.

    • Currently the only value this can be set to is pixelmania. This mode will force the numeral system to be in Hexadecimal by loading the interactive in hex and hiding the radio buttons that allow the user to change from hex to decimal and vice versa. It will also show the pixelmania logo at the top of the interactive.
  • Show the full hex or rgb code in the coloured box. This is added for all modes - not just for pixelmania. We should also consider adding this to the CMY Mixer so they are consistent. I didn't add it in this PR since this work is urgent and it's not a copy-paste job. I have made an issue for this - see Make CMY Mixer consistent with the RGB Mixer #1306

  • Add in URL redirects specified in Add URL redirects for Pixelmania #1303

  • Make the interactive look nicer on different screen sizes

Note: I had to do a hacky looking thing in urls.py to avoid the line being too long... let me know if you have a better solution!

Resolves #1303

Copy link
Contributor

@eAlasdair eAlasdair left a comment

First comment is a 'please be careful' rather than a 'please change this' because I don't know exactly what is desired. The rest is non-essential but should be done

csfieldguide/general/urls.py Outdated Show resolved Hide resolved
@@ -4,11 +4,14 @@
{% load static %}

{% block html %}
<div id="interactive-rgb-mixer" class="mb-3 container-fluid">
<div id="interactive-rgb-mixer" class="my-3 container-fluid">
<img id="pixelmania-logo" src="{% static 'interactives/rgb-mixer/img/pixelmania-logo-half-size.png' %}" class="d-none img-fluid" crossorigin="anonymous">
Copy link
Contributor

@eAlasdair eAlasdair Jul 30, 2020

Choose a reason for hiding this comment

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

Note for future: we should put this picture in a shared folder somewhere so we don't need three slightly different copies

useHex = (urlParameters.getUrlParameter('hex') || 'false') == 'true';
$("input[id='hex-colour-code']").prop('checked', useHex);
$("input[id='dec-colour-code']").prop('checked', !useHex);
}
Copy link
Contributor

@eAlasdair eAlasdair Jul 30, 2020

Choose a reason for hiding this comment

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

Note for the future: Hiding the radio buttons should be its own url parameter

Copy link
Contributor

@eAlasdair eAlasdair left a comment

Will review properly in a bit

@@ -17,7 +17,7 @@
background: rgb(127, 127, 127);
color: rgb(127, 127, 127);
border: 1px solid black;
Copy link
Contributor

@eAlasdair eAlasdair Jul 31, 2020

Choose a reason for hiding this comment

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

Suggested change
border: 1px solid black;
border: 1px solid black;

@@ -17,7 +17,7 @@
background: rgb(127, 127, 127);
Copy link
Contributor

@eAlasdair eAlasdair Jul 31, 2020

Choose a reason for hiding this comment

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

Suggested change
background: rgb(127, 127, 127);
background: rgb(127, 127, 127);

Copy link
Contributor

@eAlasdair eAlasdair left a comment

The lack of any variation in pixelmania-3 when it zooms in is potentially very confusing.

I'm fine to approve it as is but perhaps it'd be better if the image were mirrored and cropped, so a bit of it's face and beak might make it into the zoomed image

image

i.e. this
image

Copy link
Contributor

@eAlasdair eAlasdair left a comment

Not sure what's happening but I understand this needs to be released by tomorrow. Therefore I'll approve it now, any other changes can be made later if need be

@JackMorganNZ JackMorganNZ merged commit 741010d into develop Aug 1, 2020
5 checks passed
@JackMorganNZ JackMorganNZ mentioned this pull request Aug 1, 2020
@uccser-bot uccser-bot mentioned this pull request Aug 5, 2020
@JackMorganNZ JackMorganNZ deleted the rgb-mixer-pixelmania branch Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Add URL redirects for Pixelmania
3 participants