Closed nasaownsky closed 1 week ago
Hi @mxosman @lilidworkin! This is initial changes, I'll add "select" components later in this PR. For now please check my changes and tell me your thoughts, thank you!
Edit: also lint don't like if there is 2024 year in file headers, should I just replace it with 2023?
Hi @nasaownsky - thank you for your work on this! Will dive into this at some point today!
Sorry @nasaownsky! I thought I could get to this today, but will give it my full attention first thing Monday. Hope you have a restful weekend in the meantime!
Edit: also lint don't like if there is 2024 year in file headers, should I just replace it with 2023?
Yeah, we gotta update the template to 2024... For now, I think yarn lint --fix
will resolve this.
Hello @mxosman! Added requested changes, please check them out!
Also, regarding
The dropdown feels a bit narrow - can we atleast widen it to show the "Search for Agency" placeholder text?
I'm actually not sure what this dropdown should do. I can search by child agencies, and when clicking one the table should update according to the description in Figma. But what updated table should look like? Just one child agency?
I'm currently tweaking our existing dropdown to use main label as search input as per mocks, but maybe we can use search input similar to ChildAgenciesDropdown
?
Hello @mxosman! Added requested changes, please check them out!
Also, regarding
The dropdown feels a bit narrow - can we atleast widen it to show the "Search for Agency" placeholder text?
I'm actually not sure what this dropdown should do. I can search by child agencies, and when clicking one the table should update according to the description in Figma. But what updated table should look like? Just one child agency?
I'm currently tweaking our existing dropdown to use main label as search input as per mocks, but maybe we can use search input similar to
ChildAgenciesDropdown
?
Ah - yeah, that's a great question! I looked at it again and was confused myself. I put a question out in the Figma to Forrest for clarification. I wonder if it would be simpler to just have that be a search input instead of a dropdown that filters the tables live. Will follow up with you when I hear back - but I'm personally leaning towards it being a plain old search/filter input w/ no dropdown.
Thank you so much for addressing all the feedback on this, @nasaownsky! It looks great to me so far! Let me know if you'd rather open separate PRs for the dropdowns and we can gate this new component for now to local and staging environments and merge this one in.
I wonder if it would be simpler to just have that be a search input instead of a dropdown
That's actually a super good idea, I like it! But let's see what Forrest will answer to your question.
Let me know if you'd rather open separate PRs for the dropdowns
Yeah, let's merge it if everything is ok!
Oh do you mind updating the description to replace "closes #1349" to "contributes to #1349" just to save us from closing the ticket?
Description of the change
Added new table, pagination and api logic
Type of change
Related issues
contributes to #1349
Checklists
Development
This box MUST be checked by the submitter prior to merging:
These boxes should be checked by the submitter prior to merging:
Code review
These boxes should be checked by reviewers prior to merging: