Created
April 25, 2017 17:03
-
-
Save dmitru/c14889c2c9e435677785d5443c8762ae to your computer and use it in GitHub Desktop.
Feedback on Keksobooking
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
js/forms.js:12: var syncValues = function (element, value) { // TODO: make it a normal function instead of "var foo = function" expression | |
js/forms.js:16: var syncValueWithMin = function (element, value) { // TODO: make it a normal function instead of "var foo = function" expression | |
js/filter.js:11: var checkboxNames = ['wifi', 'dishwasher', 'parking', 'washer', 'elevator', 'conditioner']; // TODO: Name constants LIKE_THIS | |
js/filter.js:81: window.filter = function (offerList) { // TODO: move assignment to window outside of the module | |
js/load.js:8: var onError = function (errorMessage) { // TODO: make it a normal function instead of "var foo = function" expression, name it as an event handler | |
js/load.js:29: xhr.timeout = 10000; // 10s TODO: Name a constant LIKE_THAT | |
js/map.js:5: var updatePins = function (offers) { // TODO: make it a normal function instead of "var foo = function" expression | |
js/map.js:9: var onLoad = function (data) { // TODO: make it a normal function instead of "var foo = function" expression | |
js/photos.js:15: window.uploadPhotos = function (chooser, preview) { // TODO: Don't assign to window inside modules! Return it from module instead and assign outside | |
js/photos.js:28: reader.addEventListener('load', function () { // TODO: name the event handler function like "onSomethingHappened" ) (see nameing guides) | |
js/photos.js:34: reader.addEventListener('load', function () { // TODO: name the event handler function like "onSomethingHappened" ) (see nameing guides) | |
js/photos.js:38: for (var n = 0; n < photoArray.length; n++) { // TODO: Rename "n" to "i" | |
js/pinmove.js:7: var pinHeight = 94; // TODO: rename all constants LIKE_THIS | |
js/pinmove.js:15: formAddress.addEventListener('keydown', function (evt) { // TODO: name the event handler function like "onSomethingHappened" ) (see nameing guides) | |
js/pinmove.js:24: pinHandle.addEventListener('mousedown', function (evt) { // TODO: name the event handler function like "onSomethingHappened" ) (see nameing guides) | |
js/pinmove.js:32: var onMouseMove = function (moveEvt) { // TODO: make it a normal function instead of "var foo = function" expression | |
js/pinmove.js:64: var onMouseUp = function (upEvt) { // TODO: make it a normal function instead of "var foo = function" expression | |
js/render-pin.js:6: // TODO: Rename "i" to something meaningful, e.g. "pinId" | |
js/render-pin.js:13: pin.id = 'pin-' + i; // TODO: pass in the id from the calling code, don't concatenate strings here, just assign the passed pinId | |
js/render-pin.js:23: // TODO: Rename argument to "offers" | |
js/render-pin.js:24: window.render = function (offerList) { // TODO: Don't assign to window inside modules! Return it from module instead and assign outside | |
js/show-card.js:5: window.showCard = function (pinElements, offerList) { // TODO: rename to "offers" | |
js/show-card.js:9: var escKeyCode = 27; // TODO: Rename with CAPITAL_LETTERS | |
js/show-card.js:19: function typeReplaceRus(type) { // TODO: rename to "replaceTypeRus": verb comes first | |
js/show-card.js:29: function createImg(photos) { // TODO: Rename to "photo": it's not an array, is it? | |
js/show-card.js:37: function makeOfferTemplate(element) { // TODO: rename element to "offer" - it's not a DOM element | |
js/show-card.js:57: function closeDialog() { // TODO: rename to "onCloseDialogCLick" | |
js/show-card.js:71: function openPopupDialog(evt) { // TODO: Rename to onPinElementClick | |
js/show-card.js:80: makeOfferTemplate(offer), // TODO: pass in offer.offer, since you don't use other fields inside the function | |
js/show-card.js:93: pinElements[i].addEventListener('keydown', function openPopupOnEnterPress(evt) { // TODO: Move & rename the handler so that it's similar to other handlers | |
js/utils.js:6: return evt.keyCode === 13; // TODO: Make 13 a constant: ENTER_KEY_CODE | |
js/utils.js:12: randomArr: function (arr, length) { // TODO: rename to "getRandomArray", rename argument to "array" | |
js/utils.js:36: contains: function (arr, elem) { // TODO: rename arguments to "array", "element" |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment