Improved UX for Admin Notification Page #490
Conversation
…enhanged ux/ui of admin/notifications page
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
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.
|
…lex/ryan/ux-admin-notification-page
….com/LetsGetTechnical/gridiron-survivor into alex/ryan/ux-admin-notification-page
…the value of the selected radio button.
…radiogroup and messages on notifications page
|
This screen should have the ability to All users, users in a specific league only, etc. |
@shashilo this was discussed previously in how we want to handle it. I can grab the drop-down we have on the homepage if you'd like. Or, my original thought was that it would be based on what league was chosen in the AdminMenu on the top of the admin layout. I also have another task that was to create the league choice interface.
|
…lex/ryan/ux-admin-notification-page
…rom sendEmailUsers to groupUsers
…dd state to control which "group" value is displayed when the user makes a radio button selection
…lex/ryan/ux-admin-notification-page
| NEXT_PUBLIC_APPWRITE_API_URL= | ||
| NEXT_PUBLIC_APPWRITE_PROJECT_ID= | ||
| NEXT_PUBLIC_APPWRITE_DATABASE_ID= | ||
| APPWRITE_API_KEY= |
There was a problem hiding this comment.
Q: Why was this deleted?
| } catch (error) { | ||
| console.error('Error Sending Email:', error); |
There was a problem hiding this comment.
nit: Please remove console.error as throw new Error stack already takes care of this. And update the argument in throw new Error accordingly
| const fetchData = async (): Promise<void> => { | ||
| await getLeagueData(); | ||
| }; |
There was a problem hiding this comment.
Q: How come we don't just move the getLeagueData() function logic inside fetchData()?
| * Focuses the textarea. | ||
| * @returns {void} | ||
| */ | ||
| focus: (): void => textAreaRef.current?.focus(), |
There was a problem hiding this comment.
Q: How come we're using the ? operator here if textAreaRef.current is defined as HTMLTextAreaElement, but not HTMLTextAreaElement | undefined?
| textAreaRef.style.height = `${scrollHeight + offsetBorder}px`; | ||
| } | ||
| } | ||
| }, [textAreaRef, triggerAutoSize]); |
There was a problem hiding this comment.
Q: If we remove textAreaRef and/or triggerAutoSize, does this useEffect logic still function as intended?
| if (textAreaRef) { | ||
| if (init) { | ||
| textAreaRef.style.minHeight = `${minHeight + offsetBorder}px`; | ||
| if (maxHeight > minHeight) { | ||
| textAreaRef.style.maxHeight = `${maxHeight}px`; | ||
| } |
There was a problem hiding this comment.
nit: There's a lot of nesting here, can we not combine textAreaRef and init into the same conditional if statement?
| }); | ||
|
|
||
| it('respects minHeight and maxHeight props', () => { | ||
| render(<Textarea minHeight={100} maxHeight={200} />); |
There was a problem hiding this comment.
Q: Shouldn't this be 102px if you're testing for the min height to be 102px?
| }); | ||
|
|
||
| it('auto resizes based on content', () => { | ||
| render(<Textarea minHeight={100} maxHeight={200} />); |
There was a problem hiding this comment.
Q: Shouldn't this be 102px if you're testing for the min height to not be 102px?
| }); | ||
|
|
||
| it('updates value when props.value changes', () => { | ||
| const { rerender } = render(<Textarea value="Initial value" />); |
There was a problem hiding this comment.
nit: this variable name seems misleading if we're just rendering Textarea here, please feel free to correct me here.
This drop down is just a quick switch dropdown. It should'n't be tied to page itself. Think of it as a nav. |
…lex/ryan/ux-admin-notification-page
|
This is not the ideal functionality. We're going to merge this, but create another ticket to refactor the UX to be correct. |
shashilo
merged commit a0a2c1c
into
alex/implement-email-notification-functionality
Closes #474
Enhance the user experience of the existing admin notification page to improve its formatting and usability.
Acceptance Criteria:
Video Demo
GIS.Notifcaion.UX.Demo.mp4