Skip to content

Conversation

@avivtur
Copy link
Collaborator

@avivtur avivtur commented Nov 12, 2025

📝 Links

https://issues.redhat.com/browse/MTV-3743

📝 Description

🎥 Demo

edit-settings.mp4

📝 CC://

@mmenestr please review

@avivtur avivtur changed the title Standardize edit flow 2 Standardize edit forklift settings Nov 12, 2025
@avivtur
Copy link
Collaborator Author

avivtur commented Nov 12, 2025

Wait for #2106 to be merged first

@codecov-commenter
Copy link

codecov-commenter commented Nov 12, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 0% with 219 lines in your changes missing coverage. Please review.
✅ Project coverage is 6.81%. Comparing base (13484d0) to head (ab11ab7).
⚠️ Report is 893 commits behind head on main.

Files with missing lines Patch % Lines
...etails/tabs/Hooks/components/HookEdit/HookEdit.tsx 0.00% 36 Missing ⚠️
...overview/tabs/Settings/components/SettingsEdit.tsx 0.00% 35 Missing ⚠️
...overview/tabs/Settings/components/SettingsCard.tsx 0.00% 31 Missing ⚠️
.../tabs/Hooks/components/HookSection/HookSection.tsx 0.00% 23 Missing ⚠️
src/plans/details/tabs/Hooks/PlanHooksPage.tsx 0.00% 10 Missing ⚠️
src/components/headers/SectionHeadingWithEdit.tsx 0.00% 9 Missing ⚠️
...abs/Settings/components/EditControllerCPULimit.tsx 0.00% 7 Missing ⚠️
.../Settings/components/EditControllerMemoryLimit.tsx 0.00% 7 Missing ⚠️
...s/Settings/components/EditInventoryMemoryLimit.tsx 0.00% 7 Missing ⚠️
...iew/tabs/Settings/components/EditMaxVMInFlight.tsx 0.00% 7 Missing ⚠️
... and 11 more
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #2111       +/-   ##
==========================================
- Coverage   36.81%   6.81%   -30.00%     
==========================================
  Files         158    1014      +856     
  Lines        2548   19679    +17131     
  Branches      599    3923     +3324     
==========================================
+ Hits          938    1341      +403     
- Misses       1428   18332    +16904     
+ Partials      182       6      -176     

☔ 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.

@avivtur avivtur force-pushed the standardize-edit-flow-2 branch from 64a4db6 to ab11ab7 Compare November 12, 2025 15:02
@sonarqubecloud
Copy link

@avivtur avivtur changed the title Standardize edit forklift settings Resolves: MTV-3743 | Standardize edit forklift settings Nov 12, 2025
@mmenestr
Copy link
Collaborator

Looks good! Just a few things:

  1. Can we increase the spacing between the "Settings" header and the first field to be 16px
  2. In the modal, the header should be "Edit settings" with a lower case S

Potentially open a second PR for this if you want to keep the scope of this one simple but:
3. Ideally, there should still be tooltips/popovers in the details view. Would it be possible to make it so that each input label has a dashed underline, and if the user clicks on it they would see the tooltip that they see in the modal if they click on the question circle? So in the details view they can find a tooltip with a dashed underline, and in the modal it's still with the question circle

Also tagging @heyethankim in case you want to review this too since it's your settings page :)

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