Enhancement/64656 need to allow table actions#4498
Conversation
Refactored layout to group ComboBoxes with "..." buttons for value expression editing. Added buttons and handlers for column value and "Where" column title fields to open ValueExpressionEditorPage. Improved margins and consistency in XAML.
WalkthroughActTableEditPage redesigns column, row, and where-clause selector controls to use ChangesActTableEditPage Expression Editor Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitignore:
- Line 337: The current .gitignore entry lists the specific generated filename
"Ginger_dsbf4hog_wpftmp.csproj"; change that rule to a wildcard pattern that
covers all temporary WPF project files (e.g., replace the specific filename
entry with a pattern like "*_wpftmp.csproj" or "**/*_wpftmp.csproj") so future
variants don’t get committed.
In `@Ginger/Ginger/Actions/ActionEditPages/ActTableEditPage.xaml.cs`:
- Around line 797-809: After ValueExpressionEditorPage closes in
ColumnValueVE_Click and WhereColumnTitleVE_Click, programmatic updates to
cmbColumnValue.Text and WhereColumnTitle.Text don't trigger downstream
recalculation; after setting the Text and updating mAct, call the component
methods that recompute derived state (e.g., UpdateRelatedActions() and
SetDescriptionDetails()) so the UI and dependent logic refresh immediately.
Locate ColumnValueVE_Click and WhereColumnTitleVE_Click (they instantiate
ValueExpressionEditorPage and assign cmbColumnValue.Text / WhereColumnTitle.Text
from mAct) and add calls to UpdateRelatedActions() and SetDescriptionDetails()
right after those assignments to ensure state is refreshed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 998c6f99-dbe9-4f1d-96cd-4d6a8693f482
📒 Files selected for processing (3)
.gitignoreGinger/Ginger/Actions/ActionEditPages/ActTableEditPage.xamlGinger/Ginger/Actions/ActionEditPages/ActTableEditPage.xaml.cs
| /Ginger/GingerTest/GingerTest.GeneratedMSBuildEditorConfig.editorconfig | ||
| *.DS_Store | ||
| /Ginger-Automation/Ginger | ||
| /Ginger/Ginger/Ginger_dsbf4hog_wpftmp.csproj |
There was a problem hiding this comment.
Generalize the temporary WPF project ignore pattern.
This ignore rule is too specific to one generated filename; future *_wpftmp.csproj variants may still leak into git history. Prefer a stable wildcard pattern for this temp artifact class.
Suggested change
-/Ginger/Ginger/Ginger_dsbf4hog_wpftmp.csproj
+/Ginger/Ginger/*_wpftmp.csproj📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /Ginger/Ginger/Ginger_dsbf4hog_wpftmp.csproj | |
| /Ginger/Ginger/*_wpftmp.csproj |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.gitignore at line 337, The current .gitignore entry lists the specific
generated filename "Ginger_dsbf4hog_wpftmp.csproj"; change that rule to a
wildcard pattern that covers all temporary WPF project files (e.g., replace the
specific filename entry with a pattern like "*_wpftmp.csproj" or
"**/*_wpftmp.csproj") so future variants don’t get committed.
Up to standards ✅🟢 Issues
|
Added SetDescriptionDetails() and conditional UpdateRelatedActions() calls to ColumnValueVE_Click and WhereColumnTitleVE_Click handlers. This ensures UI description details and related actions are updated after editing value expressions, improving consistency in the ActTableEditPage.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/Ginger/Actions/ActionEditPages/ActTableEditPage.xaml.cs (1)
790-795: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winApply the same state-refresh pattern for consistency.
The
RowSelectorValueVE_Clickhandler updates the text field but doesn't refresh derived state, similar to the issue that was just fixed inColumnValueVE_ClickandWhereColumnTitleVE_Click. For consistency, consider adding the same refresh calls here.♻️ Suggested consistency fix
private void RowSelectorValueVE_Click(object sender, RoutedEventArgs e) { ValueExpressionEditorPage w = new ValueExpressionEditorPage(mAct, ActTableElement.Fields.LocateRowValue, Context.GetAsContext(mAct.Context)); w.ShowAsWindow(eWindowShowStyle.Dialog); RowSelectorValue.Text = mAct.LocateRowValue; + SetDescriptionDetails(); + if (eBaseWindow.Equals(BaseWindow.WindowExplorer)) + { + UpdateRelatedActions(); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Ginger/Ginger/Actions/ActionEditPages/ActTableEditPage.xaml.cs` around lines 790 - 795, The RowSelectorValueVE_Click handler updates RowSelectorValue.Text but doesn't refresh derived UI/state; modify ActTableEditPage.RowSelectorValueVE_Click to invoke the same refresh/update calls used in ColumnValueVE_Click and WhereColumnTitleVE_Click after setting mAct.LocateRowValue/RowSelectorValue.Text (i.e., copy the same RefreshControls/RefreshBindings/UpdateDerivedState method calls those handlers use) so the derived state is recomputed and the UI stays consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Ginger/Ginger/Actions/ActionEditPages/ActTableEditPage.xaml.cs`:
- Around line 790-795: The RowSelectorValueVE_Click handler updates
RowSelectorValue.Text but doesn't refresh derived UI/state; modify
ActTableEditPage.RowSelectorValueVE_Click to invoke the same refresh/update
calls used in ColumnValueVE_Click and WhereColumnTitleVE_Click after setting
mAct.LocateRowValue/RowSelectorValue.Text (i.e., copy the same
RefreshControls/RefreshBindings/UpdateDerivedState method calls those handlers
use) so the derived state is recomputed and the UI stays consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0bdbfce-a62e-4108-adcb-0e7f4f8dba10
📒 Files selected for processing (1)
Ginger/Ginger/Actions/ActionEditPages/ActTableEditPage.xaml.cs
Description
Type of Change
Checklist
[IsSerializedForLocalRepository]where neededReporter.ToLog()patternSummary by CodeRabbit
Release Notes
New Features
Style