Skip to content

Completed Component Library#3

Open
jugalkpatel wants to merge 83 commits into
mainfrom
development
Open

Completed Component Library#3
jugalkpatel wants to merge 83 commits into
mainfrom
development

Conversation

@jugalkpatel
Copy link
Copy Markdown
Owner

Guys, Please Review My Component Library.

@asmitzz
Copy link
Copy Markdown

asmitzz commented Apr 18, 2021

image
fixed the snakbar heading in the sidebar

@jugalkpatel
Copy link
Copy Markdown
Owner Author

image
fixed the snakbar heading in the sidebar

image
do you have zoomed screen. looks good in mine.

Copy link
Copy Markdown

@thesudeshdas thesudeshdas left a comment

Choose a reason for hiding this comment

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

I've suggested some changes, make them and you can merge the development branch to the main branch

Comment thread index.html
@@ -0,0 +1,1606 @@
<!DOCTYPE html>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

mobile responsive view is breaking, the right margin is more than left margin

Comment thread index.html
<body id="body">
<div class="main">
<!-- start of the navbar -->
<nav class="navbar">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Navigation brand name is breaking in mobile view

Comment thread index.html Outdated
</span>
Lorem ipsum
<span class="product-info__small"
>Lorem ipsum dolor sit Lorem</span
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

indentation for the span

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread index.html Outdated
<div
class="container--border container--margin responsive--container"
>
<div class="flex--container">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

some classes have -- in between, whereas some have __, is it a convention?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

yes, -- is a modifier, __ is a block

Comment thread index.html Outdated
</p>
</div>
<footer class="modal__article__footer">
<button
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

hovering on the button makes the button same color as that of background. Make it a different color for readability

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Done.

Comment thread landing-page.html Outdated
@@ -0,0 +1,58 @@
<!DOCTYPE html>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I couldn't access this landing page from anywhere.

Comment thread index.html Outdated
<!-- start of the container -->
<div class="container">
<!-- start of the sidebar-->
<div class="sidebar">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

sidebar doesn't appear on mobile view

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

yet not implement for mobile.

Comment thread index.html
<body id="body">
<div class="main">
<!-- start of the navbar -->
<nav class="navbar">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

make navbar sticky? it'll look much better and it's better for navigation through the site

Comment thread index.html Outdated
height="32"
/>
</div>
<pre>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

prism code is having a scrollbar on the right side, unnecessary

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed.

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.

4 participants