search

CU-CommunityApps / cwd_project

A starter child theme for cwd_base, providing just the basics for beginning a custom Drupal design.
1 stars 0 forks source link

searchfix/issue8 #3

Closed philwilliammee closed 4 years ago

philwilliammee commented 4 years ago

This pull request fixs issue https://github.com/CU-CommunityApps/cwd_project/issues/2 and addresses issue https://github.com/CU-CommunityApps/cwd_base/issues/8

(Also, whitespace tweaks.)

alisonjo315 commented 4 years ago

oops, commented on the wrong element -- "looking, thanks!"

alisonjo315 commented 4 years ago

It looks great! I want there to be a note "on the record" that there were whitespace tweaks, too, and I just learned that I can update the PR description haha, so I did that :)

Love the "default" in the switch, good thinking. Question: "project search," I'm not sure that phrase is meaningful, OR maybe it just means something I'm not aware of. (Oh and, probably add a period at the end of the message, I think......?) console.warn($(this).val() + " not found in project search");

alisonjo315 commented 4 years ago

(meanwhile)

@philwilliammee The code from project.js works great on the new FCS site! Aside from the thoughts in my prior comment, it all looks good to me!

philwilliammee commented 4 years ago

For the whitespaces I have my IDE set up to use drupal code standards. https://www.drupal.org/docs/develop/standards/javascript/javascript-coding-standards

All code MUST indent using two (2) space characters,

if your using anything outside of the drupal code standards it should be noted in the doc block.

alisonjo315 commented 4 years ago

Awesome, thanks!

(AFAIK the exception is composer.json and related files -- but that should be in editorconfig or whatever it uses.)