Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

10460 story #5542

Merged
merged 88 commits into from
Nov 15, 2024
Merged

10460 story #5542

merged 88 commits into from
Nov 15, 2024

Conversation

nechama-krigsman
Copy link
Contributor

@nechama-krigsman nechama-krigsman commented Nov 13, 2024

John Cruz added 30 commits October 22, 2024 12:42
…ime a filter is modified; Added pagination to table;
}}
>
{defaultValue && (
<option key={defaultValue.label} value={defaultValue.value}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Can we ensure label will always be unique?

Copy link
Contributor

@Mwindo Mwindo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, @cruzjone-flexion and @nechama-krigsman! Thank you!

I left several minor comments. I'm not sure any of these need to be addressed in the current PR, especially since my plan is to open up a follow-up "clean-up" PR anyway to make sure my work in 10461 is consistent with yours. (E.g., I handled the "current as of" differently, and it's probably better to be consistent.)

Comment on lines +162 to +173
const publicTrialsSessionUpdateFormValueSequence = (
...args: Parameters<typeof updateFormValueSequence>
) => {
if (displayProgressSpinnerSequence)
displayProgressSpinnerSequence({ timeInSeconds: 0.25 });
updateFormValueSequence(...args);
updateFormValueSequence({
key: 'pageNumber',
root: ROOT,
value: 0,
});
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be done in Cerebral?

Copy link
Contributor

@cruzjone-flexion cruzjone-flexion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All feedback is refactoring related, we will handle it all in a separate PR.

@jimlerza jimlerza merged commit 4d7be07 into ustaxcourt:staging Nov 15, 2024
44 checks passed
@jimlerza jimlerza deleted the 10460-story branch November 15, 2024 18:05
@jimlerza jimlerza mentioned this pull request Nov 15, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants