Skip to content

Design Changes - Brush Buttons, Layout, Color#2

Open
aswathirazak1 wants to merge 2 commits intodash1291:masterfrom
aswathirazak1:master
Open

Design Changes - Brush Buttons, Layout, Color#2
aswathirazak1 wants to merge 2 commits intodash1291:masterfrom
aswathirazak1:master

Conversation

@aswathirazak1
Copy link
Copy Markdown

hey, please review code?

@aswathirazak1 aswathirazak1 changed the title First Pull Request Design Changes - Brush Buttons, Layout, Color Jun 11, 2019
Comment thread style.css
box-shadow: 4px 10px 14px 0px rgba(168,137,26,0.4);
}


Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

extra blank lines could be removed here

Comment thread style.css

.toolbox__item:hover {
box-shadow: 4px 10px 14px 0px rgba(0,0,0,0.15);
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

indentation issue. 4 spaces before box-shadow instead of 8. } should be shifted to the beginning of the line

Comment thread style.css
position:absolute;
left:50px;
bottom: 50px;
width:1600px;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this is too wide. we might have to think about how to make it scale according to the viewport width until some minimum width. narrower than that will qualify for some mobile effort which could be done later

Comment thread style.css
align-items: center;
#key-selection-area {
position:absolute;
left:50px;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

space after :

Comment thread style.css
font-family: "Benton Sans", "Helvetica Neue", helvetica, arial, sans-serif;
margin: 2em;
background-color: #5c5470;
background-image: linear-gradient(100deg, #8256D6, #5943C8, #3C39BC);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

looks nice!

Comment thread drawing.js

var tool = 0 // 0 = pen, 1 = eraser


Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

we should avoid extraneous changes

Comment thread drawing.js
viz.text('Draw something here', WIDTH / 2 - 150, HEIGHT / 2)
viz.textSize(20)
viz.fill(300)
viz.text('Scribble, quibble, dribble, you know what to do. Hit spacebar when you’re done.', WIDTH / 2 - 300, HEIGHT / 2)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

lol

Comment thread drawing.js
viz.fill(150)
viz.text('Draw something here', WIDTH / 2 - 150, HEIGHT / 2)
viz.textSize(20)
viz.fill(300)
Copy link
Copy Markdown
Owner

@dash1291 dash1291 Jun 11, 2019

Choose a reason for hiding this comment

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

fill() takes color as a parameter. 300 means gray value = 300 which is out of bounds (0-255 being the permissible range). check this out - https://p5js.org/learn/color.html

Comment thread drawing.js
viz.fill(150)
viz.text('Preparing stuff', WIDTH / 2 - 100, HEIGHT / 2)
viz.textSize(20)
viz.fill(300)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

fill() takes color as a parameter. 300 means gray value = 300 which is out of bounds (0-255 being the permissible range). check this out - https://p5js.org/learn/color.html

Comment thread style.css
color: #333;
background: #dbd8e3;
background: #FF90C3;
box-shadow: 4px 10px 14px 0px rgba(168,137,26,0.4);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

spaces after commas :)

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.

2 participants