Skip to content

Frequency Analysis Widget UI Redesign#4000

Merged
Hamms merged 8 commits into
stagingfrom
frequency-trirow-dragging
Sep 18, 2015
Merged

Frequency Analysis Widget UI Redesign#4000
Hamms merged 8 commits into
stagingfrom
frequency-trirow-dragging

Conversation

@Hamms

@Hamms Hamms commented Sep 16, 2015

Copy link
Copy Markdown
Contributor

Significantly redesigns the Frequency Analysis Widget.

Before

image

After

image

Major changes of note:

  • Removed input box, replaced with dropdown + dialog (similar to text compression widget)
  • Rather than swapping substitutions, you now have a bank of unassigned letters you can drag up and assign
  • Controls are all isolated into a tabular area, you can now sort message frequency letters and substitution letters independently

@Hamms
removed (hid) input box, added a dropdown for messages select and a m…
4355685 
…odal for new input. Also added a legend
@Hamms
HUGE refactor; splits frequency chart into two
5634727 
We now have three interactable rows and two full charts. The top chart
and top two rows are essentially what we had before, the bottom chart
and third now are a new "unassigned letters" category.

This means that we no longer have to rely on just switching to assign
substitutions; we actually start with nothing assigned, and drag
assignments up from row 3 to row 2
@Hamms
Multiple layout changes
6049497 
- turned control toggle buttons into tabs
- moved message data sort into tab
- moved legend to above graph and replaced HTML h4 with SVG text
- generalized sorting
@Hamms
removed input, replaced it with a BarGraph.message property
dbbced3
@Hamms
JSDoc updates and rearrangement
c30e3fb
@Hamms
Adds independent Message and Substitution sorting
95bf88b
@joshlory

joshlory commented Sep 16, 2015

Copy link
Copy Markdown
Contributor

Initial impressions:

Awesome redesign! This is way easier to understand when solving "Sample Message (hard)". Love the dragging of the blue final letters instead of the orange starting letters.

A few nits:

  1. Can we bring back the locks? I miss being able to lock letters I'm sure of while I experiment with other substitutions.

  2. "Random" here is a little confusing. Elijah had to explain that this means "Random Cipher". "Sort" was a lot clearer as a user.

    screen shot 2015-09-16 at 12 21 50 pm

  3. Prefer "Sort originals" above "Sort substitutions" since it's a closer match to how the graphs are originally organized.

@Hamms

Hamms commented Sep 16, 2015

Copy link
Copy Markdown
Contributor Author

@sfilman thoughts on #2?

@Hamms

Hamms commented Sep 16, 2015

Copy link
Copy Markdown
Contributor Author

How does everyone feel about something like this:

image
image

joshlory
joshlory reviewed Sep 16, 2015
View reviewed changes
Comment thread dashboard/public/frequency/frequency.js Outdated

@joshlory joshlory Sep 16, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: indentation.

@joshlory

joshlory commented Sep 16, 2015

Copy link
Copy Markdown
Contributor

LGTM. Lucky PR #4000! 🏆

Feel free to ignore any or all nits.

@sfilman

sfilman commented Sep 16, 2015

Copy link
Copy Markdown
Contributor

Chatted with @Hamms we're going to go with Caesar substitution and Random substitution to match the Lesson Plan & Activity Guide

Hamms added a commit that referenced this pull request Sep 18, 2015
@Hamms
Merge pull request #4000 from code-dot-org/frequency-trirow-dragging
f91a018 
Frequency Analysis Widget UI Redesign
@Hamms Hamms merged commit f91a018 into staging Sep 18, 2015
@Hamms Hamms deleted the frequency-trirow-dragging branch November 3, 2015 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Hamms @joshlory @sfilman