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

Initial Merge #1

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Initial Merge #1

wants to merge 21 commits into from

Conversation

Atishay-J
Copy link
Owner

Open for PR review
CSS component library
Deployed at: https://metaphorui.netlify.app/

@Atishay-J Atishay-J changed the title Development Initial Merge Mar 26, 2021
Copy link

@vaishnavme vaishnavme left a comment

Choose a reason for hiding this comment

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

Great design. Really liked code snippet toggling feature.
For for list, input and navigation component assign code snippet is wrong.

  • In Input Comp: Form Input showing code snippet of list with icons
  • In List Comp: For ordered list: Showing code snippet of vertical navigation.
  • In Navigation: For navigation bar: Showing code snippet of Avatar

Thats all i got. Everything else works perfect.

Components/MetaphorUI.css Outdated Show resolved Hide resolved
Components/inputs.html Outdated Show resolved Hide resolved
Components/lists.html Outdated Show resolved Hide resolved
Components/Navigation.html Outdated Show resolved Hide resolved
Copy link

@villdev villdev left a comment

Choose a reason for hiding this comment

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

Great job! Something I noticed is you've mixed rem and px a lot. As far as possible go for rem units. A few accessibility suggestions below.

Components/inputs.html Show resolved Hide resolved
Components/MetaphorUI.css Show resolved Hide resolved
Components/Alerts.html Show resolved Hide resolved
Copy link

@amansethi00 amansethi00 left a comment

Choose a reason for hiding this comment

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

Great work Atishay!I have suggested some changes for code consistency and user feedback.

Components/Alerts.html Show resolved Hide resolved
</div>
<div class="alert-cont alert">
<ion-icon name="alert-circle-sharp" class="alert-icon"></ion-icon>
<p class="alert-text">Warning</p>

Choose a reason for hiding this comment

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

accessibility of alert text(success,info and warning) is not optimal,I would suggest go with colors of alert from material ui

class="avatar-img"
src="../Assests/Profile Images/woman profile 2.jpg"
/>
<ion-icon

Choose a reason for hiding this comment

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

ion-icon for badges should have a border,try border of 2px maybe

Components/Buttons.html Outdated Show resolved Hide resolved
Vitamin C serum for Healthy and good looking skin..
</p>
</div>
<button class="card-btn">Add to Cart</button>

Choose a reason for hiding this comment

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

button do not give feedback to user,maybe add box-shadow on hover

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