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

popup project #75

Merged
merged 7 commits into from
Aug 15, 2022
Merged

popup project #75

merged 7 commits into from
Aug 15, 2022

Conversation

NatalyaBortsova
Copy link
Contributor

@NatalyaBortsova NatalyaBortsova commented Aug 5, 2022

HTML і CSS практика: Hooli-style Popup

Demo |
Code base

The code is submitted in a dedicated feature branch.

Only code files are submitted.

Please, review.

Copy link
Collaborator

@A-Ostrovnyy A-Ostrovnyy left a comment

Choose a reason for hiding this comment

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

Good work!
Please add source(not minified) file of styles.

submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Show resolved Hide resolved
Copy link
Collaborator

@A-Ostrovnyy A-Ostrovnyy left a comment

Choose a reason for hiding this comment

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

Good Work!
Looks like you forget to update index.html

submissions/NatalyaBortsova/popup/css/style.scss Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/css/style.scss Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@A-Ostrovnyy A-Ostrovnyy left a comment

Choose a reason for hiding this comment

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

Good progress!
But your I can't open your popup from keyboard)

submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/css/style.scss Outdated Show resolved Hide resolved
Copy link
Collaborator

@A-Ostrovnyy A-Ostrovnyy left a comment

Choose a reason for hiding this comment

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

Two issue must be fixed)

submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@A-Ostrovnyy A-Ostrovnyy left a comment

Choose a reason for hiding this comment

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

One last issue)

submissions/NatalyaBortsova/popup/index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@A-Ostrovnyy A-Ostrovnyy left a comment

Choose a reason for hiding this comment

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

One more time)

</li>
<li class="menu__item">
<button class="menu__item-button animation" type="button">
<img class="menu__image" src="images/icons/bell.png" alt="Turn on notifications" width="25" height="25" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about these icons?

</li>
<li class="menu__item">
<button class="menu__item-button animation" type="button">
<img class="menu__image" src="images/icons/user.png" alt="Open a login window" width="30" height="30" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Be more careful in the future)

@A-Ostrovnyy A-Ostrovnyy merged commit 6374199 into kottans:main Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants