Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code Review #29

Open
AseelL opened this issue Mar 15, 2023 · 5 comments
Open

Code Review #29

AseelL opened this issue Mar 15, 2023 · 5 comments

Comments

@AseelL
Copy link

AseelL commented Mar 15, 2023

Greetings,
interesting idea, great effort and outcome, immaculate, we always love a good laugh in the hard days
I love how tidy the files are, everything around the project outlook looks nice and clean, we won't talk about the text adding capabilities since it's an extra but it was almost perfect too

Screenshot from 2023-03-15 23-05-37

@AseelL
Copy link
Author

AseelL commented Mar 15, 2023

const addListener = (selector, eventName, callback) => {
if (document.querySelector(selector) !== null)
document.querySelector(selector).addEventListener(eventName, callback);
};
const getElementById = (id) => {
const element = document.getElementById(id);
return element;
};
const querySelector = (query) => {
const element = document.querySelector(query);
return element;
};

I like these guys so much, that's some useful stuff to make, but I'd still reserve the top section of the code file for variables just to keep it conventional and clean

@AseelL
Copy link
Author

AseelL commented Mar 15, 2023

const topText = getElementById("top-text");
const bottomText = getElementById("bottom-text");
const rightTop = querySelector(".right-top");
const rightBottom = querySelector(".right-bottom");
const rightTopText = getElementById("right-top-text");
const rightBottomText = getElementById("right-bottom-text");
const memeContainer = querySelector(".overlay");
const topTextElement = querySelector(".top-text");
const bottomTextElement = querySelector(".bottom-text");
const textJoke = document.querySelector("header .random-joke");

I like the naming here so much! I'm liking what im seeing honestly

@AseelL
Copy link
Author

AseelL commented Mar 15, 2023

const createHtmlElement = (el, className, id) => {
const ele = document.createElement(el);
if (className) {
ele.className = className;
}
if (id) {
ele.id = id;
}
return ele;
};

laziness is the enemy of creativity (at least for me), el is The in french , let's not make nicknames for element, it's already elegant as it is
but i do like the idea of this as well :)

@AseelL
Copy link
Author

AseelL commented Mar 15, 2023

const testArray = [

test array shouldn't have been pushed but keep working on that functionality! you're almost there! and make sure to not push any console.log's as well, keep it clean till the end 😄

@AseelL
Copy link
Author

AseelL commented Mar 15, 2023

there's a stray image pushed into the repository as well?
I recommend installing this extension for vscode since it helps you with the spelling.

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

No branches or pull requests

1 participant