Fall 2025 updates#41
Conversation
|
@eshaan-s18 @yxli001 merging this now so we have it for orientation, if you guys find any issues lmk and I'll fix in a follow-up PR |
|
@benjaminJohnson2204 I checked for typos and unclear instructions, everything LGTM. I went over the code a little bit and so far it LGTM, I'll plan to run through the tutorial E2E when I get some time |
| 10. Add some CSS classes to `TaskList.module.css` and add the corresponding `className` props to `TaskList.tsx`. | ||
| 1. We need one class for the list title, which uses the heading font. This works similarly to the title and description classes from `TaskItem`. | ||
| 2. We need another class for the inner `<div>`, which is the item container. Use flexbox again to align its children: column direction, horizontally stretched. The item container itself should also have `width: 100%`. | ||
| 3. Finally, we need a class for the outermost list container `<div>`. This just needs a top margin of 3rem. |
There was a problem hiding this comment.
I would also add to add a class for .errorModalText to override the text color so it doesn't show white text on a white background. TaskForm.module.css has an example on how to do this
There was a problem hiding this comment.
ok so for this file there were a couple things I noticed:
- for the PUT api route sections 3 and 4, i think eslint is more strict now about unsafe assignment of an
anyvalue, so I had to make anUpdateTaskBodyobject type similar toCreateTaskBodyto fix this issue. maybe this will be worth mentioning in the instructions. when i did the onboarding repo 2 years ago i didnt have this problem but i think eslint might be more strict now. lmk if im wrong tho - For
TaskItemsection 3 when we say to do alert() if it failed, eslint doesnt like using alert I dont think so it tells me to either doconsole.error()or a custom alert. Maybe we can use Constellation Dialog here?
There was a problem hiding this comment.
this comment is for 2.3 so not this file:
For the update to component TaskForm section:
For the part about updating the third test to use the correct mock. I also noticed that we have to update expect(updateTask).toHaveBeenCalledWith({ because the component expects more fields than just title and description for update. I might be wrong with my implementation so lmk if I'm wrong. If this is the case though maybe we can add to update toHaveBeenCalledWith with the necessary fields for the test
Various improvements for Fall 2025:
react-helmet-asyncis no longer necessary)Dialogcomponent to display error messages, as well as adding a section about Constellation in the docsupdateTask,cURLnot working on Windows, using React hooks incorrectly)In reviewing this, would appreciate double-checking for typos, unclear instructions, or anything breaking when running through the tutorial E2E (if yall have time). Thanks!