update MCP update/install modal primary button with correct text color#10166
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR updates the custom Install and Update MCP modal primary button label, shortcut icon, and shortcut border colors to use a foreground selected for the accent background.
Concerns
- No blocking correctness or security concerns found.
- The Update modal custom label still bypasses disabled button text styling; see inline suggestion.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
…warpdotdev/warp into andy/fix-mcp-modal-button-color
acarl005
requested review from
kevinchevalier and
peicodes
and removed request for
kevinchevalier and
vkodithala
vkodithala
left a comment
vkodithala
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the screenshot + context. Left a minor comment, feel free to implement as you see fit.
| // Disable the update button if no updates are selected | ||
| let has_selection = self.selected_updates.iter().any(|&x| x); | ||
| let label_color = if has_selection { | ||
| appearance.theme().font_color(appearance.theme().accent()) |
There was a problem hiding this comment.
nit, non-blocking: Looks like we call appearance.theme() a bunch and I wonder if this is necessary. Can we just extract this into a local variable like we do on :85?
There was a problem hiding this comment.
good call. done in both here and the installation modal
#10166) ## Description Fixes: #10517 This PR fixes the primary buttons on the MCP modals for install and update to use the same text color as other primary buttons. See here for the "source of truth": https://github.com/warpdotdev/warp/blob/10b2540c1d493a1869949d5f5a7d69a7b0c8d377/app/src/view_components/action_button.rs#L1229-L1240 ## Testing ### Before <img width="2560" height="1528" alt="Screenshot 2026-05-05 101722" src="https://github.com/user-attachments/assets/c2cc4523-eefc-40ac-b8ac-86d9bfc813e3" /> ### After <img width="1920" height="1200" alt="Screenshot 2026-05-05 105447" src="https://github.com/user-attachments/assets/1df0419f-a270-4e15-be19-a33758791622" />
#10166) ## Description Fixes: #10517 This PR fixes the primary buttons on the MCP modals for install and update to use the same text color as other primary buttons. See here for the "source of truth": https://github.com/warpdotdev/warp/blob/10b2540c1d493a1869949d5f5a7d69a7b0c8d377/app/src/view_components/action_button.rs#L1229-L1240 ## Testing ### Before <img width="2560" height="1528" alt="Screenshot 2026-05-05 101722" src="https://github.com/user-attachments/assets/c2cc4523-eefc-40ac-b8ac-86d9bfc813e3" /> ### After <img width="1920" height="1200" alt="Screenshot 2026-05-05 105447" src="https://github.com/user-attachments/assets/1df0419f-a270-4e15-be19-a33758791622" />
#10166) ## Description Fixes: #10517 This PR fixes the primary buttons on the MCP modals for install and update to use the same text color as other primary buttons. See here for the "source of truth": https://github.com/warpdotdev/warp/blob/10b2540c1d493a1869949d5f5a7d69a7b0c8d377/app/src/view_components/action_button.rs#L1229-L1240 ## Testing ### Before <img width="2560" height="1528" alt="Screenshot 2026-05-05 101722" src="https://github.com/user-attachments/assets/c2cc4523-eefc-40ac-b8ac-86d9bfc813e3" /> ### After <img width="1920" height="1200" alt="Screenshot 2026-05-05 105447" src="https://github.com/user-attachments/assets/1df0419f-a270-4e15-be19-a33758791622" />
2 participants
Description
Fixes: #10517
This PR fixes the primary buttons on the MCP modals for install and update to use the same text color as other primary buttons.
See here for the "source of truth":
warp/app/src/view_components/action_button.rs
Lines 1229 to 1240 in 10b2540
Testing
Before
After