Skip to content

Workspace Factory #4: Category Color#24

Merged
edauterman merged 4 commits intoworkspacefactory_masterfrom
workspacefactory4_color
Jul 26, 2016
Merged

Workspace Factory #4: Category Color#24
edauterman merged 4 commits intoworkspacefactory_masterfrom
workspacefactory4_color

Conversation

@edauterman
Copy link
Copy Markdown
Owner

@edauterman edauterman commented Jul 25, 2016

The user can now pick a color for their category given a choice of colors. The user chooses colors through the Closure PopupColorPicker, which is initialized with Blockly specific colors, and their choice of colors shows up as a left border on the category tab in the view (to look similar to the Blockly toolbox UI).

Also made the few small changes suggested by picklesrus and vicng in the last CL.
color screenshot


This change is Reviewable

Emma Dauterman added 2 commits July 25, 2016 13:29
@edauterman
Copy link
Copy Markdown
Owner Author

+@picklesrus +@vicng +@quachtina96


Review status: 0 of 6 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@picklesrus
Copy link
Copy Markdown
Collaborator

:lgtm:


Review status: 0 of 6 files reviewed at latest revision, 1 unresolved discussion.


demos/workspacefactory/index.html, line 510 [r2] (raw file):

  goog.events.listen(popupPicker, 'change', function(e) {
        controller.changeColor(popupPicker.getSelectedColor());
      });

I think indentation is off here.


Comments from Reviewable

@vicng
Copy link
Copy Markdown
Collaborator

vicng commented Jul 25, 2016

:lgtm:


Review status: 0 of 6 files reviewed at latest revision, 2 unresolved discussions.


demos/workspacefactory/factory_controller.js, line 344 [r2] (raw file):

 * a valid CSS string.
 */
FactoryController.prototype.changeColor = function(color) {

Nit: Rename changeSelectedCategoryColor

I think changeColor is a bit too vague since this controller is basically the only entry point that the app is talking to.


Comments from Reviewable

@edauterman
Copy link
Copy Markdown
Owner Author

Review status: 0 of 6 files reviewed at latest revision, 2 unresolved discussions.


demos/workspacefactory/factory_controller.js, line 344 [r2] (raw file):

Previously, vicng wrote…

Nit: Rename changeSelectedCategoryColor

I think changeColor is a bit too vague since this controller is basically the only entry point that the app is talking to.

Changed.

demos/workspacefactory/index.html, line 510 [r2] (raw file):

Previously, picklesrus wrote…

I think indentation is off here.

We talked about this and no changes necessary.

Comments from Reviewable

@edauterman edauterman merged commit a3b69ee into workspacefactory_master Jul 26, 2016
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.

4 participants