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

Tan 3487/screening status filter item #9994

Merged
merged 15 commits into from
Jan 15, 2025

Conversation

brentguf
Copy link
Contributor

@brentguf brentguf commented Jan 7, 2025

There's a lot of code specifically for status code prescreening in the generic status filter, which makes it hard to reason about/messy. So I'm creating a separate component for the prescreening status code to take this out.

This is prep work for next ticket(s). No functional changes in here. I might further refactor this (reduce code duplication) later if I have time left.

Changelog

Changed

  • Remove hover styles from Screening status in input manager so it doesn't look as if you can drop inputs on this status.

Copy link

@brentguf brentguf marked this pull request as draft January 7, 2025 16:31
@cl-dev-bot
Copy link
Collaborator

cl-dev-bot commented Jan 8, 2025

Warnings
⚠️

The changelog is empty. What should I put in the changelog?

⚠️ The PR title contains no Jira issue key (case-sensitive)
Messages
📖 Notion issue: TAN-3487
📖

Run the e2e tests

📖 Check translation progress

Generated by 🚫 dangerJS against f8833d6

Comment on lines +65 to +69
const prescreeningButtonIsDisabled =
!phasePrescreeningEnabled || !preScreeningFeatureAllowed;

const prescreeningTooltipIsDisabled =
phasePrescreeningEnabled && preScreeningFeatureAllowed;
Copy link
Contributor Author

@brentguf brentguf Jan 8, 2025

Choose a reason for hiding this comment

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

I've removed the status code checks here since we know code is always prescreening.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the original StatusFilter with all the prescreening status related code that I'd like to separate from all other status filters. Only changes where indicated. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed all prescreening specific code from this.

@brentguf brentguf marked this pull request as ready for review January 8, 2025 10:11
@amanda-anderson amanda-anderson removed their request for review January 8, 2025 10:58
@brentguf brentguf requested a review from IvaKop January 8, 2025 12:05
Copy link
Contributor

@IvaKop IvaKop left a comment

Choose a reason for hiding this comment

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

There are still a couple of more things that can be cleaned up from the prescreening status component before it's fully complete.

@brentguf brentguf mentioned this pull request Jan 14, 2025
@brentguf brentguf merged commit 542127f into master Jan 15, 2025
10 checks passed
@brentguf brentguf deleted the TAN-3487/screening-status-filter-item branch January 15, 2025 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants