Skip to content

[Task-6] As a user, I can view the sign in page#39

Open
gutakk wants to merge 65 commits intodevelopfrom
feature/task-6-sign-in-frontend
Open

[Task-6] As a user, I can view the sign in page#39
gutakk wants to merge 65 commits intodevelopfrom
feature/task-6-sign-in-frontend

Conversation

@gutakk
Copy link
Owner

@gutakk gutakk commented Jun 11, 2021

Resolve #4

What happened 👀

  • Style the sign in page following the Figma design
  • Add necessary attributes to HTML element
  • Add a necessary component (Alert, AuthLogo and Background)
  • Add necessary env to test workflow
  • Add UI test using Cypress
  • Update README

Insight 📝

Mock the HTTP request in the UI test using cy.intercept() and add the mock response to fixtures

Proof Of Work 📹

@gutakk gutakk added this to the 0.2.0 milestone Jun 11, 2021
@gutakk gutakk self-assigned this Jun 11, 2021
@gutakk gutakk linked an issue Jun 11, 2021 that may be closed by this pull request
@gutakk gutakk requested a review from rafayet-monon June 14, 2021 11:19
@gutakk gutakk marked this pull request as ready for review June 14, 2021 11:19
@gutakk gutakk force-pushed the feature/task-6-sign-in-frontend branch 4 times, most recently from c2ca26a to 0573ad8 Compare June 15, 2021 07:58
@gutakk gutakk requested a review from carryall June 16, 2021 04:56
@gutakk gutakk force-pushed the feature/task-6-sign-in-frontend branch from 0573ad8 to 3152481 Compare June 16, 2021 05:36
@gutakk
Copy link
Owner Author

gutakk commented Jun 16, 2021

@rafayet-monon Sorry I didn't notice Cypress convention. I will try to update the code to follow the convention.

@gutakk gutakk requested a review from rafayet-monon June 16, 2021 09:33
@gutakk gutakk force-pushed the feature/task-6-sign-in-frontend branch 2 times, most recently from f3da2dd to a7ea289 Compare June 17, 2021 08:44
@gutakk gutakk requested a review from rafayet-monon June 17, 2021 11:24
@gutakk gutakk added the @0.3.0 label Jun 18, 2021
@gutakk gutakk modified the milestones: 0.2.0, 0.3.0 Jun 18, 2021
Base automatically changed from feature/task-5-sign-in-backend to develop June 18, 2021 08:36
@gutakk gutakk force-pushed the feature/task-6-sign-in-frontend branch from 46d23a2 to 4da5629 Compare June 18, 2021 09:22
return (
<div className="auth-logo">
<img className="auth-logo__image" src={logoWhite} alt="Nimble" />
<p className="auth-logo__label">{label}</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@gutakk Not sure why do you desire to put this here, IMO this is not quite related to the logo but more like a page title or a form title. So how about moving it out of this component?
Screen Shot 2564-06-18 at 19 05 59

Copy link
Owner Author

Choose a reason for hiding this comment

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

Because I think all auth page have the same layout that's why I named this component as AuthLogo but maybe we can change to AuthTitle to make it clearer. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gutakk How about page-heder or auth-header, since the title sounds more like the component going to cover only the title? 🤔

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sounds great. Fixed in 1323a67

@gutakk gutakk added the @0.4.0 label Jun 21, 2021
@gutakk gutakk modified the milestones: 0.3.0, 0.4.0 Jun 21, 2021
@gutakk gutakk requested a review from carryall June 21, 2021 08:06
Copy link
Collaborator

@carryall carryall left a comment

Choose a reason for hiding this comment

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

@gutakk One last concern seems like the font-weight does nothing. These two labels with font-weight 800 and 400 look exactly the same 😅

<label class="form-input-group__label" for="password">Password</label>
<label class="form-input-group__label" for="password" style="
    font-weight: 400;
">Password</label>

Screen Shot 2564-06-28 at 19 36 36

You might need to convert the font face to web fonts to fix it. Here's the generator that can help you, at least provide woff woff2 you can take a look at the example from The1 project

return (
<div className="auth-logo">
<img className="auth-logo__image" src={logoWhite} alt="Nimble" />
<p className="auth-logo__label">{label}</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@gutakk How about page-heder or auth-header, since the title sounds more like the component going to cover only the title? 🤔

@gutakk gutakk requested a review from carryall June 29, 2021 07:03
@gutakk
Copy link
Owner Author

gutakk commented Jun 30, 2021

@gutakk One last concern seems like the font-weight does nothing. These two labels with font-weight 800 and 400 look exactly the same 😅

<label class="form-input-group__label" for="password">Password</label>
<label class="form-input-group__label" for="password" style="
    font-weight: 400;
">Password</label>

Screen Shot 2564-06-28 at 19 36 36

You might need to convert the font face to web fonts to fix it. Here's the generator that can help you, at least provide woff woff2 you can take a look at the example from The1 project

Fixed in 810a1d6

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.

[Task-6] As a user, I can view the sign in page

3 participants