-
Notifications
You must be signed in to change notification settings - Fork 222
Adds Hill Climbing Search diagram for chapter 4 #80
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
Conversation
| .attr('y', (d) => this.h - this.yScale(d.objective)) | ||
| .style('opacity', (d) => (d.visited) ? 1 : 0) | ||
| .style('fill', colors.hill) | ||
| .style('border', '1px solid'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think border won't do anything for svg; you can use stroke instead.
| .style('opacity', (d) => { | ||
| return (d.visited) ? 1 : 0.6; | ||
| }) | ||
| .style('fill', (d) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Color implementation suggestion:
Right now you have some colors defined in a color object and other colors defined in the page CSS.
Instead of setting the fill color here, use .attr('class', 'visited') or .classed('visited', true), and then you'll be able to set all the fill colors in the page CSS, in one place. That also will make it easier to override the colors for one diagram while reusing code across diagrams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would be a much better design.
| constructor(selector, h, w) { | ||
| this.h = h; | ||
| this.w = w; | ||
| this.svg = d3.select(selector).html("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the .html("") for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.html("") is used here to delete the previously added elements. It is applicable when 'restart' is pressed and a new instance of the class is created.
|
Cool, generators! Color suggestions:
Specifically, GRMA and maxima in the new diagram: GRMA is big so it should be less bold; the maxima are small and important so they could be more bold. Maybe try |
|
Updated as per suggestions. |
This commit