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

tasks done by Karam Karim #4

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

tasks done by Karam Karim #4

wants to merge 11 commits into from

Conversation

BlackGhost3
Copy link

No description provided.

@BlackGhost3 BlackGhost3 changed the title task 1 & 2 done by Karam Karim task 1 & 2 & 3 done by Karam Karim Apr 6, 2018
<body>

<div class="container">
<ui class="dropdown" data-text="Select Language">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this tag ui?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo, It meant to be ul not ui.

*/
setTimeout(a => location.reload(true), 2000);

};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but you don't need js here. Used it locally

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I added this my mistake, It won't be here on next push.

width: 70%;
}

.container > .body > .langs > span {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can instead make this shorter ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could add a class or id to the element to target them directly, the reason that I made it this way it's easier to read for me and understand what i'm styling, but if you don't like this type of selectors I can change it 😃

margin: 20px;
}

.container > .header > h2, .container > .header > p{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same goes here, you can make this selector shorter as well

margin: .5em 1em;
}

.container > .body > .langs > span > label{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

cursor: pointer;
}

.container > .body > .langs > span > input[type="checkbox"]:checked + label{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here too

padding: 2em 1em;
}

.container > .body > .buttons > a{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too and check others as well

list-style: none;
position: relative;
border-radius: 5px;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are missing shadow in dropdown

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, sorry about this one it was 2 am and I didn't notice it in the pics.

@BlackGhost3 BlackGhost3 self-assigned this Apr 8, 2018
@BlackGhost3 BlackGhost3 changed the title task 1 & 2 & 3 done by Karam Karim tasks done by Karam Karim Apr 8, 2018
@BlackGhost3 BlackGhost3 removed their assignment Apr 8, 2018
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.

2 participants