Danielle/535 add current entry number in header of entry page #615
Conversation
|
Gridiron Survivor Application
Project name: Gridiron Survivor Application
Only deployments on the production branch are activated automatically. If you'd like to activate this deployment, navigate to your deployments. Learn more about Appwrite Function deployments.
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…n-survivor into danielle/535-add-current-entry-number-in-header-of-entry-page
…ildren in the middle
| Week {week} pick | ||
| </h1> | ||
|
|
||
| <Heading |
There was a problem hiding this comment.
Why not test that H1 is on the screen and has the correct value as well?
There was a problem hiding this comment.
@ryandotfurrer How come all headings do not have pb-8 by default? Heading's usually have margin from the next element.
There was a problem hiding this comment.
@shashilo complete
| <Heading | ||
| as='h2' | ||
| className='pb-8 text-muted-foreground' | ||
| data-testid='entry-name' |
There was a problem hiding this comment.
Let's follow BEM naming convention. This should be data-testid='week__entry-name'
There was a problem hiding this comment.
@shashilo complete
…header-of-entry-page
| as='h2' | ||
| className='pb-8 text-muted-foreground' | ||
| data-testid='week__entry-name' | ||
| >{entryName} |
There was a problem hiding this comment.
Sorry if this is a dumb question, but is there a reason we're not just using currentEntry.name here instead of creating state for it? It never changes, right?
There was a problem hiding this comment.
@mhchen currenEntry.name is inside of a function call and thus not available to the greater Week component. That's why I saved that piece of data to state when the function is called. I was not 100% confident in this approach though so please feel free to dissect this!
There was a problem hiding this comment.
Oh nope, you’re right. Turns out it was a dumb question!
I’m so used to a different data fetching paradigm that I forgot this is necessary sometimes.
There was a problem hiding this comment.
Mike does bring up a good point here. This is not part of this ticket, but because we're now tracking 2 pieces of data from the response object, we should refactor the state to store the pick history and the entry name. A good ticket to refactor some code into the backlog.
…header-of-entry-page
Danielle254
deleted the
danielle/535-add-current-entry-number-in-header-of-entry-page
branch
Before: no entry number in header of entry page
After: entry number added to header of entry page
<Heading/>component<Heading/>componentColor is up for debate as is spacing.