tristanjacobs89 / wafs Goto Github PK

View Code? Open in Web Editor NEW

1.0 1.0 1.0 1.29 MB

Repo for Web App From Scratch course @ HvA Minor Web Development

JavaScript 86.20% HTML 7.80% CSS 6.00%

wafs's People

Contributors

Stargazers

Watchers

Forkers

pierman1

wafs's Issues

Code of conduct

please don't refer to fascist regimes in code

https://github.com/SadisticSun/web-app-from-scratch/blob/123bcd52765bac5e252b2d91a66c2ee217681785/static/js/app.js#L9

Unnecessary second XMLHttpRequest in your getDetailsFile

Your first XMLHttpRequest in doApiCall returns 25 objects with information you can use in the gif-details page. No need to do a second API call just to retrieve these information again.

It's too much to explain but basically you can save the data that you need for each gif file in the app var in a seperate app.date object.

I've refactored the code:

(function() {
    "use strict";

/* UTILITIES
========================================================================= */

    var utils = {
        $: function(id) {
            return document.querySelector(id);
        },
        $$: function(el) {
            return document.querySelectorAll(id);
        }
    };

/* DOM ELEMENTS
========================================================================= */

    var elements = {
        sections: {
            $details:           utils.$("#details"),
            $img_container:     utils.$('#img-container'),
            $img_source:        utils.$('#img-source'),
            $img_post_date:     utils.$('#img-posted-on'),
            $start:             utils.$('#start'),
            $result_area:       utils.$('#results'),
            $error:             utils.$('#error')
        },

        form: {
            $self:              utils.$('#search-form'),
            $search_input:      utils.$('#search-input'),
            $gif_selector:      utils.$('#gif-selector'),
            $sticker_selector:  utils.$('#sticker-selector'),
            $submit_button:     utils.$('#submit'),
            $spinner:           utils.$('#spinner')
        },

        toggleSpinner: function() {
            elements.form.$spinner.classList.toggle('hidden');
            elements.form.$submit_button.classList.toggle('hidden');
        }
    };

/* ROUTES
========================================================================= */

    var routes = {
        init: function() {
            routie({

                // Set default route to #start
                '': function() {
                    location.hash = '#start';
                },
                // Start route
                'start': function() {
                    elements.sections.$start.classList.remove("hidden");
                    elements.sections.$details.classList.add("hidden");

                },
                // Details route
                'details/:id': function() {
                    elements.sections.$start.classList.add("hidden");
                    elements.sections.$details.classList.remove("hidden");
                    app.getDetailsAndRender();
                }
            });
        }
    };

/* MAIN APP
========================================================================= */
    var app = {

        /* Initializer
        ===================== */

        init: function() {
            routes.init();

            elements.form.$self.addEventListener('submit', function(e) {
                // Prevent page from reloading on submit
                e.preventDefault();

                // Get the url parameters
                var query_type  = app.getQueryType(),
                    query       = app.getSearchQuery(),
                    urlForApi   = api_config.URL + query_type + "/search?q=" + query + "&api_key=" + api_config.KEY;

                // Get data from the API
                app.doApiCall(urlForApi);
            });
        },

        /* Data retrieved by api cal
        ===================== */

        data: {},

        /* Call to API
        ===================== */

        doApiCall: function(url) {

            // Empty the result area
            elements.sections.$result_area.innerHTML = '';
            // Toggle the loading spinner
            elements.toggleSpinner();

            // Set a 1 second delay on the API call for perceived performance
            setTimeout(function() {
                var request = new XMLHttpRequest();
                request.open('GET', url, true);

                // empty the data object
                app.data = {};


                request.onload = function() {
                    if (request.status >= 200 && request.status < 400) {
                        // Use reduce to filter out only the needed information from the returned data
                        var response = JSON.parse(request.responseText).data.reduce(
                          function(accumulator, currentValue) {
                            accumulator.push({
                              id: currentValue.id,
                              image: currentValue.images.preview_webp,
                            });

                            // populate app.data with values for the gif-info page
                            app.data[currentValue.id] = {
                              bigImg: currentValue.images.downsized_large.url,
                              postDate: currentValue.import_datetime,
                              source: currentValue.source_post_url
                            }

                            return accumulator;

                          // Sort the data by their ID's
                        }, []).sort(function(a, b) {
                          return a.id - b.id;
                        });

                        // Check if the response object has data
                        if (response.length == 0) {

                          console.log('No data..');
                          elements.toggleSpinner();

                          // Show error message on page if there is no data
                          elements.sections.$error.classList.remove('hidden');

                        } else {

                          elements.sections.$error.classList.add('hidden');
                          console.log('Data found!');
                          elements.toggleSpinner();

                          // Call the render data function to show images
                          app.renderData(response);
                        }
                    } else {
                        console.log("Error!");
                    }
                };
                request.onerror = function(err) {
                    console.log('Error making connection: ' + err);
                };
                request.send();
            }, 1000)
        },

        /* Get the query type (stickers or gifs)
        ===================== */

        getQueryType: function() {

          var queryType;

            // Set the query type to "gifs" if gifs checkbox is selected
            if (elements.form.$gif_selector.checked) {
                queryType = "gifs";
                console.log('User selected GIFS');
                return queryType;

                // Set the query type to "stickers" if stickers checkbox is selected
            } else if (elements.form.$sticker_selector.checked) {
                queryType = "stickers";
                console.log('User selected STICKERS');
                return queryType;
            }
        },

        /* Get the search query
        ===================== */

        getSearchQuery: function() {
            var query = elements.form.$search_input.value;
            return query;
        },

        /* Render data on page
        ===================== */

        renderData: function(results) {
            results.map(function(item) {

                var img_link  = item.image.url;
                var item_id   = item.id;

                window.scroll(0, 300);
                elements.sections.$result_area.innerHTML +=
                '<a class="animated fadeIn" href="#details/' + item_id + '">' +
                    '<img src="' + img_link + '"/>' +
                '</a>';
            });
        },

        /* Get details of selected image
        ===================== */

        getDetailsAndRender: function() {
            var hashRoute = location.hash;
            var id        = hashRoute.slice(9);
            var gifObj    = app.data[id];

            // get data for the gif-element from app.data object
            var bigImg    = gifObj.bigImg;
            var postDate  = gifObj.postDate;
            var source    = gifObj.source;

            elements.sections.$img_container.innerHTML  = '<img class="detail-img" src="' + bigImg + '">';
            elements.sections.$img_source.innerHTML     = '<a href="'+ source + '" target="_blank">' + source + '</a>';
            elements.sections.$img_post_date.innerHTML  = postDate;

        }
    };
    app.init();
})();

Can't find the refactored geo script

Route for detail view is missing

https://github.com/SadisticSun/web-app-from-scratch/blob/123a41b8dc8091cbf41f64f6e3eb350e7f49ace5/opzet%20spa/static/js/app.js#L26

Uncompressed backgroud image

The background image in your spa is almost 4 mb and takes 10 seconds to load on a speedy connection. Please compress the image

ul element can only contain li elements

https://github.com/SadisticSun/web-app-from-scratch/blob/123a41b8dc8091cbf41f64f6e3eb350e7f49ace5/opzet%20spa/index.html#L22

Don't just return user provided input value

Always check user input before making a call to the server. Now, if I push spacebar into the searchfield (thus, technically not empty), a call is made to the server but it's an unnecessary call.

        getSearchQuery: function() {
            var query = elements.form.$search_input.value;
            return query;
        },

Double var declaration

There is no lexical scoping in If-statements, so no need to declare a var 2 times.

// Set the query type to "gifs" if gifs checkbox is selected
            if (elements.form.$gif_selector.checked) {
                var queryType = "gifs";
                console.log('User selected GIFS');
                return queryType;

                // Set the query type to "stickers" if stickers checkbox is selected
            } else if (elements.form.$sticker_selector.checked) {
                var queryType = "stickers";
                console.log('User selected STICKERS');
                return queryType;
            }

Instead do:

            var queryType;
            
            // Set the query type to "gifs" if gifs checkbox is selected
            if (elements.form.$gif_selector.checked) {
                queryType = "gifs";
                console.log('User selected GIFS');
                return queryType;

                // Set the query type to "stickers" if stickers checkbox is selected
            } else if (elements.form.$sticker_selector.checked) {
                queryType = "stickers";
                console.log('User selected STICKERS');
                return queryType;
            }

cache DOM elements in variables

https://github.com/SadisticSun/web-app-from-scratch/blob/123a41b8dc8091cbf41f64f6e3eb350e7f49ace5/opzet%20spa/static/js/app.js#L28

Only use div element for styling purposes

Use main element Instead of div.container element?

https://github.com/SadisticSun/web-app-from-scratch/blob/123a41b8dc8091cbf41f64f6e3eb350e7f49ace5/opzet%20spa/index.html#L27

Add these to your issue list?

label nice-to-have?
https://github.com/SadisticSun/web-app-from-scratch/blob/123a41b8dc8091cbf41f64f6e3eb350e7f49ace5/opzet%20spa/static/js/app.js#L16

App needs more structure

Separate logic into object literals and try to keep your methods lean and mean (as little code as possible per method)

Bind events instead of adding event as a property

AddEventListener is prefered over onhashchange..

Function declarations on top and at the bottom

Because you're using function declarations (difference in hoisting vs function expressions), you can order them at the bottom of the file for better structure and readability.

// code...

// bottom of file ...
function $id(id) { ... }
function getSearchResults(filter)  { ... }

Lean and mean HTML

Don't use unnecessary container

elements. Try to keep your html as lean and mean as possible.

Better handling of server output

When you search for "dog eat", the server responds a second gif object where somehow the image object is undefined and thus preventing the rest of the gifs to load.

I've updated the code like this:

// this
var img_link  = item.image.url;
var item_id   = item.id;

// to
var img_link  = item.image ? item.image.url : "";
var item_id   = item.id;

You can see now in the console that indeed the second gif object has an undefined image property instead of an object. But the rest is loaded like it should.

Use a predefined selectvalue.

If user doesn't select either the sticker or gif radio button, the server responds with a 404 error that is not handled by the app. Instead the spinner keeps on spinning.

XMLHttpRequest cannot load https://api.giphy.com/v1/undefined/search?q=cats&api_key=dc6zaTOxFJmzC&limit=25. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'null' is therefore not allowed access. The response had HTTP status code 404.

Solution: either preselect either 'gif' or 'sticker' radio button to prevent this error or refactor your formElements.selectedFilter() function that can handle the exception when none of the radiobuttons is selected.

Unused function $$(id)

Following function is unused in your app.js file.

    function $$(id) {
        return document.querySelectorAll(id);
    }

Placement <script> tag

Place <script> tag above the closing tag underneath all other elements. Or place it in the section with a defer attribute

Keep app.js only for the app function

Relocate utils, elements and routes to its own files for better structure, like you did with the apiconfig.js file.

I would even go further and move each method in your app object to a different file. But thats just a personal style and I also don't always do it myself.

Suggestion: Declare vars at the top in functions as well

https://github.com/SadisticSun/WAFS/blob/master/static/js/app.js#L141

Maybe something like:

var _html = '',
    _gifs = document.querySelectorAll('.resultgif');

Use 'submit' event to submit form

https://github.com/SadisticSun/web-app-from-scratch/blob/123a41b8dc8091cbf41f64f6e3eb350e7f49ace5/opzet%20spa/static/js/app.js#L43

Don't use 'userscalable = no'

Zooming is an important accessibility feature that is used by a lot of people.

Don't use inline styles in script

https://github.com/SadisticSun/web-app-from-scratch/blob/123a41b8dc8091cbf41f64f6e3eb350e7f49ace5/opzet%20spa/static/js/app.js#L26

strict mode

Are you sure you want to run strict mode on the global scope?

Add .gitignore file to prevent uploading node-modules folder

Als je geen .gitignore file aanmaakt voor js apps, dan zullen ook je node-modules folders geupload worden naar github. (api opdracht folder)

Wrong param passed in function

See $$ method.

var utils = {
        $: function(id) {
            return document.querySelector(id);
        },
        $$: function(el) {
            return document.querySelectorAll(id);
        }
    };

Use better error handling

At this moment, when server responds with a 400+ error, the app just console.logs "Error!". Make better use of the errors received by the server for your users so they know what's going on when something goes wrong.

Thus refactor your request.onload method, especially this line:

else {
    console.log("Error!");
}