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

Mm 005 header v2 #7

Merged
merged 18 commits into from
Jul 31, 2024
Merged

Mm 005 header v2 #7

merged 18 commits into from
Jul 31, 2024

Conversation

Purbai
Copy link
Collaborator

@Purbai Purbai commented Jul 31, 2024

Description of changes -

added a button in the style of hamburger in header
when button is clicked, outputs msg to console log
added unit test for onclick

Screenshots - for changes to the UI

image

image

image

Things to Check

  • Have all the requirments of your ticket have been met
  • Have you added tests if needed for your ticket
  • Are all the tests passing

Copy link
Contributor

@Kiran-R-K Kiran-R-K left a comment

Choose a reason for hiding this comment

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

Really great work - super clear to read and nice testing
I have a couple of formatting comments but otherwise it's looking great - please let me know when it's ready for re-review

src/App.tsx Outdated
function App() {
return (
<div className="App">
<Router>
<h1>MarsioKart</h1>
<div><Header/></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

If you remove the <div>s from this line and just have <Header/> it will be easier for a screen reader to see what is happening and translate it - please can you do that?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is looking great and is nice and clear - in order to prevent the src folder from being too full of separate files (and therefore harder to read through) please can you create a Hamburgerbutton folder and put this file and Hamburgerbusston.tsx in it

src/header.tsx Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

As above - please can you also create a Header folder and put this file in it

@Kiran-R-K Kiran-R-K mentioned this pull request Jul 31, 2024
3 tasks
@Purbai Purbai requested a review from Kiran-R-K July 31, 2024 12:11
Copy link
Contributor

@Kiran-R-K Kiran-R-K left a comment

Choose a reason for hiding this comment

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

Really good work

@Purbai Purbai merged commit 9dd8173 into main Jul 31, 2024
1 check passed
@Purbai Purbai deleted the mm-005-header-v2 branch July 31, 2024 12:40
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.

3 participants