Skip to content

Conversation

@Oksamies
Copy link
Contributor

This issue is caused by bad Radix code. Root cause can be found here radix-ui/primitives#3759
The fix is quite temporary and ideally should be fixed by refactoring
select to be a homebaked component

@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

Walkthrough

This pull request refactors the member addition form from a traditional form-based submission model to a modal-driven approach. The MemberAddForm component restructures its UI layout, introducing dedicated field containers and moving submission logic to an onClick handler. The underlying useStrongForm hook is simplified by replacing its useCallback wrapper with a plain function and consolidating error handling. Additionally, the Select component's trigger button receives an explicit type="button" attribute to prevent unintended form submission behavior.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: addressing a role select issue when adding team members, which aligns with the primary changes across the modified files.
Description check ✅ Passed The description correctly identifies the issue, provides context about the root cause (Radix UI bug), and notes it's a temporary workaround pending refactoring.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor Author

Oksamies commented Jan 22, 2026

<NewButton
type="submit"
csVariant="accent"
onSubmit={strongForm.handleSubmit}
Copy link

Choose a reason for hiding this comment

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

The onSubmit prop is passed to NewButton, but since the surrounding <form> element has been removed, this event handler will never be triggered. Buttons do not fire submit events on click; they fire click events. To fix this, change onSubmit to onClick so the form submission logic executes when the button is clicked.

          onClick={strongForm.handleSubmit}
Suggested change
onSubmit={strongForm.handleSubmit}
onClick={strongForm.handleSubmit}

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Oksamies fix this

@codecov
Copy link

codecov bot commented Jan 22, 2026

Codecov Report

❌ Patch coverage is 16.66667% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 15.63%. Comparing base (f0d9285) to head (ee42016).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
...settings/teams/team/tabs/Members/MemberAddForm.tsx 0.00% 54 Missing ⚠️
...ges/cyberstorm/src/newComponents/Select/Select.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1698      +/-   ##
==========================================
- Coverage   15.64%   15.63%   -0.02%     
==========================================
  Files         326      326              
  Lines       23232    23230       -2     
  Branches      721      721              
==========================================
- Hits         3635     3632       -3     
- Misses      19597    19598       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@VilppeRiskidev VilppeRiskidev left a comment

Choose a reason for hiding this comment

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

Fix the graphite comment

This issue is caused by bad Radix code. Root cause can be found here radix-ui/primitives#3759
The fix is quite temporary and ideally should be fixed by refactoring
select to be a homebaked component
@Oksamies Oksamies force-pushed the 01-22-fix_role_select_not_working_when_adding_a_team_member branch from 6f778ba to ee42016 Compare January 27, 2026 13:18
@Oksamies
Copy link
Contributor Author

Noticed that the handleSubmit had a unnecessary useCallBack so I removed it; https://github.com/thunderstore-io/thunderstore-ui/pull/1698/changes#diff-006cbc2523d0fdff444bbf2e77fa941d5627ce00dccd9dbbfbf61df7c99f3ab5

(it doesn't do anything as the values it were checking against had nothing to do with the forms data fields)

@Oksamies Oksamies merged commit 258e172 into master Jan 27, 2026
29 of 31 checks passed
@Oksamies Oksamies deleted the 01-22-fix_role_select_not_working_when_adding_a_team_member branch January 27, 2026 20:57
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