Add the start of a note UI #29
Conversation
|
Some screenshots:
|
|
Now with event note display |
bherbst
left a comment
bherbst
left a comment
There was a problem hiding this comment.
Really great start - only thing I'd really like to see fixed before merging is the note type and state enum issue!
| browserName = 'Firefox'; | ||
| } else if (userAgent.includes('Safari') && !userAgent.includes('Chrome')) { | ||
| browserName = 'Safari'; | ||
| } else if (userAgent.includes('Edg')) { |
There was a problem hiding this comment.
Today I learned
Microsoft chose the Edg token to avoid compatibility issues caused by Edge string, which was previously used for the legacy Microsoft Edge browser based on EdgeHTML
| <!-- Match-specific fields --> | ||
| {#if noteType === 'match' || (noteType === 'team' && matchNumber)} |
There was a problem hiding this comment.
I'm debating whether it makes more sense to split these out into different components or keep them together with shared saving logic. I think this is fine for now though, but I don't love the large if blocks (and somewhat murky state & component parameters since we need all of the possible pieces of state for all the possible note types defined)
| } | ||
| </script> | ||
|
|
||
| <Modal bind:open={isOpen} {title}> |
There was a problem hiding this comment.
Future enhancement - the event note modal is awkwardly narrow on large screens
| <div> | ||
| <label | ||
| for="teamNumberInput" | ||
| class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1" | ||
| > | ||
| Team Number (optional) | ||
| </label> | ||
| <input | ||
| id="teamNumberInput" | ||
| type="number" | ||
| bind:value={teamNumber} | ||
| min="1" | ||
| class={styles.input.base} | ||
| placeholder="Leave blank for match-wide note" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Is adding a match-level note with a team number something that we expect folks will want instead of just adding a note on that team in a match directly?
There was a problem hiding this comment.
Yes, that is something I expect to want. For example, recording that there was some sort of electrical failure in match 10.
There was a problem hiding this comment.
Or that some sensor miscounted.
| <div class="flex flex-wrap gap-2"> | ||
| <span class="{styles.badge.base} {styles.badge.blue}"> | ||
| {formatIssueType(note.issueType)} | ||
| </span> | ||
| <span | ||
| class="inline-flex items-center rounded-full px-2.5 py-0.5 text-xs font-medium {getStatusColor( | ||
| note.resolutionStatus | ||
| )}" | ||
| > | ||
| {formatResolutionStatus(note.resolutionStatus)} | ||
| </span> | ||
| {#if isMatchSpecific} | ||
| <span class="{styles.badge.base} {styles.badge.purple}"> | ||
| Match {note.matchNumber}{note.playNumber ? `.${note.playNumber}` : ''} | ||
| </span> | ||
| {/if} | ||
| </div> |
There was a problem hiding this comment.
I'm not sure what exactly is going wrong yet but I'm always getting the open status here, even for notes I can see have a resolved status in the database.
There was a problem hiding this comment.
Same for the issue type - maybe our enum deserialization just isn't working?
There was a problem hiding this comment.
This was an FMS bug, not a bug with the app; it was fixed for the regular 2025 release, but I haven't made a new build with the requisite changes for us yet. We can ignore.
|
Pushed another change to deal with scaling on phones; things were being pushed too wide. We likely will need some more work there, possibly pushing the node count/add button to a new line, rather than keeping it on the same line as the team number. |
2 participants
This adds some basics for fetching and displaying notes from the server. Included is note counts display, and the ability to create notes. Actual display of notes will come next.
Note: based on #28, will rebase after that is merged.