-
-
Notifications
You must be signed in to change notification settings - Fork 496
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
Adding Testing with Vite #1908
base: main
Are you sure you want to change the base?
Adding Testing with Vite #1908
Conversation
@@ -1,16 +1,23 @@ | |||
import './App.css' | |||
import 'bootstrap/dist/css/bootstrap.min.css'; | |||
import MyNavBar from './components/Navbar' | |||
import MyNavBar from './routes/Navbar' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally would leave NavBar
in the components folder because that's what NavBar is, a shared component used on multiple pages; a route would be a page/view*.
* At least in the traditional/beginner setup
<MyNavBar /> | ||
<ListStudents /> | ||
|
||
<MyNavBar handleMe={handleMe} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React convention is that prop names that are callbacks start with the word "on" and then a description of what event it is. So since this callback is called whenever the account menu item is clicked, I'd rename this prop to onAccountMenuItemClicked
or something along those lines.
And then handleMe
can be renamed to handleAccountMenuItemClicked
since "handle" is the conventional suffix used for implementations of callback props
<div id="contact"> | ||
<div> | ||
<img | ||
key={contact.avatar} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A key
prop is not necessary unless you're in a loop OR are abusing the key
prop to explicitly trigger re-renders (which doesn't look to be the case here)
</> | ||
) : ( | ||
<i>No Name</i> | ||
)}{" "} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
)}{" "} | |
)} |
</div> | ||
</div> | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
</div> | |
</div> | |
) | |
} | |
</div> | |
</div> | |
) | |
} |
/* | ||
describe('event signup form', () => { | ||
it('renders a form', () => { | ||
render(<EventSignUpForm />); | ||
|
||
const signUpForm = screen.getByTestId('event-form'); | ||
expect(signUpForm).toBeInTheDocument(); | ||
}); | ||
}); | ||
|
||
|
||
test('test that MyNavBar renders', () => { | ||
// const navbarElement = render(<MyNavBar/>); | ||
// const navbarId = navbarElement.getByTestId('navbar'); | ||
// expect(navbarId).toBeDefined(); | ||
// }); | ||
|
||
|
||
|
||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* | |
describe('event signup form', () => { | |
it('renders a form', () => { | |
render(<EventSignUpForm />); | |
const signUpForm = screen.getByTestId('event-form'); | |
expect(signUpForm).toBeInTheDocument(); | |
}); | |
}); | |
test('test that MyNavBar renders', () => { | |
// const navbarElement = render(<MyNavBar/>); | |
// const navbarId = navbarElement.getByTestId('navbar'); | |
// expect(navbarId).toBeDefined(); | |
// }); | |
*/ |
Looks like unused code?
// /* | ||
// const router = createBrowserRouter( | ||
// createRoutesFromElements( | ||
// <Route path="/" element={<App />}> | ||
// <Route path="login" element={<Contact />} /> | ||
// </Route> | ||
// ) | ||
// ); | ||
|
||
// */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused code to prevent confusion OR write an explanation on what this commented out code does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A key prop is only essential if you're working within a loop or intentionally using it to force re-renders, which doesn’t seem applicable in this situation.
@@ -6,7 +6,8 @@ | |||
"scripts": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can i provide here
No description provided.