Enhancement of fill percentage property accessibility#659
Conversation
Updated the `FillPercentage` property in `AlignmentSpotPropertyModel.cs` to include a setter. The setter checks for changes in value before updating `innerModel.FillParcentage` and raises the `OnPropertyChanged` event to notify listeners of the change.
Changed the `IsReadOnly` property for the "Fill %" column from `True` to `False` in several XAML files, enabling user editing of fill percentage values. Affected files include: - DimsAlignmentSpotTableView.xaml - GcmsAlignmentSpotTableView.xaml - ImmsAlignmentSpotTableView.xaml - LcimmsAlignmentSpotTableView.xaml - LcmsAlignmentSpotTableView.xaml - LcmsProteomicsAlignmentTableView.xaml
There was a problem hiding this comment.
Pull Request Overview
This pull request enables user editing of the "Fill %" column in alignment table views across multiple chromatography and mass spectrometry data types. The main changes involve converting read-only DataGrid columns to editable ones and adding a property setter to support the modification.
- Made the "Fill %" column editable in six alignment table views (LCMS, GCMS, DIMS, LCIMMS, IMMS, and proteomics)
- Added a setter implementation to the
FillPercentageproperty inAlignmentSpotPropertyModelto enable data binding updates
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| LcmsProteomicsAlignmentTableView.xaml | Changed IsReadOnly from True to False for Fill % column in proteomics alignment view |
| LcmsAlignmentSpotTableView.xaml | Changed IsReadOnly from True to False for Fill % column in LCMS alignment view |
| LcimmsAlignmentSpotTableView.xaml | Changed IsReadOnly from True to False for Fill % column in LCIMMS alignment view |
| ImmsAlignmentSpotTableView.xaml | Changed IsReadOnly from True to False for Fill % column in IMMS alignment view |
| GcmsAlignmentSpotTableView.xaml | Changed IsReadOnly from True to False for Fill % column in GCMS alignment view |
| DimsAlignmentSpotTableView.xaml | Changed IsReadOnly from True to False for Fill % column in DIMS alignment view |
| AlignmentSpotPropertyModel.cs | Converted FillPercentage from a read-only property to a property with getter and setter, including PropertyChanged notification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set { | ||
| if (innerModel.FillParcentage != value) { | ||
| innerModel.FillParcentage = (float)value; | ||
| OnPropertyChanged(nameof(FillPercentage)); | ||
| } | ||
| } |
There was a problem hiding this comment.
The FillPercentage property is calculated automatically as (float)alignedPeaks.Length / alignment.Count in DataObjConverter.SetRepresentativeProperty. Allowing manual editing could create data inconsistency where the user-entered value diverges from the actual calculated fill percentage. Consider adding input validation to ensure values are between 0 and 100, or recalculating dependent values when this property is modified manually.
| set { | |
| if (innerModel.FillParcentage != value) { | |
| innerModel.FillParcentage = (float)value; | |
| OnPropertyChanged(nameof(FillPercentage)); | |
| } | |
| } |
| get => innerModel.FillParcentage; | ||
| set { | ||
| if (innerModel.FillParcentage != value) { | ||
| innerModel.FillParcentage = (float)value; |
There was a problem hiding this comment.
The underlying property 'FillParcentage' in AlignmentSpotProperty has a spelling error ('Parcentage' instead of 'Percentage'). While this change correctly accesses the misspelled property, the underlying typo should be addressed in a future refactoring to maintain code quality.
| get => innerModel.FillParcentage; | |
| set { | |
| if (innerModel.FillParcentage != value) { | |
| innerModel.FillParcentage = (float)value; | |
| get => innerModel.FillPercentage; | |
| set { | |
| if (innerModel.FillPercentage != value) { | |
| innerModel.FillPercentage = (float)value; |
PR Classification
Enhancement of property accessibility and UI interactivity.
PR Summary
This pull request modifies the
FillPercentageproperty to allow both getting and setting, along with updating the UI to enable editing of this property in several data grid views.AlignmentSpotPropertyModel.cs: Added a setter to theFillPercentageproperty and implemented property change notification.DimsAlignmentSpotTableView.xaml,GcmsAlignmentSpotTableView.xaml,ImmsAlignmentSpotTableView.xaml,LcimmsAlignmentSpotTableView.xaml,LcvsAlignmentSpotTableView.xaml,LcvsProteomicsAlignmentTableView.xaml: ChangedIsReadOnlyfor theFill %column fromTruetoFalseto allow user edits.