Add UI to Edit Table dialog for pick once tables #5868
Merged
+509
−416
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This isn't the right situation to throw an exception, since it is at least as likely to indicate a failure for previous code to unbind the model rather than being a problem of the current caller double-binding. Throwing an exception here has the potential to crash MT and even the OS as the exception is reported in error dialogs on repeat. This case still indicates a logical error, though, so log it as such. But after that, just unbind the old model and carry on.
It also does not require the parent container name to be provided. There was some sneaky state in the `#replaceComponent()` method to to set the `#componentMap`. field. That field is now eagerly initialized in the constructor, including the component collection.
- Remove unused fields, component accessors, and unnecessary null checks - Prefer `String#isEmpty()` over comparing `String#length()` against `0` - Replace magic constants for columns with named constants, and make image-specific operations only apply to the image column regardless of order. - Move tooltip assignment into the .form file - Use ServerCommand#putLookupTable(LookupTable) to update just the one specific table instead of syncing all campaign properties. - Change the `AbeillePanel<>` model type to from `LookupTableTableModel` to `LookupTable`
- Columns names don't need to be injected since they are statically defined. - Rows are now a dedicated type rather than a list of "untyped" `String`. - `LookupTableTableModel` no longer needs to be package-private
Also make it more idiomatic: - `AbeillePanel` model type is `LookupTable`, not `LookupTableTableModel`. - Actually bind a model via `#bind()` instead of using a separate `#attach()`. - True committment via `#commit()` instead of an `ACCEPT` action listener. - Set the `Accept` button to be the default button for the dialog. - Forbid `null` bindings, instead requiring the caller to provide a new LookupTable - Create EditLookupTablePanel instances on demand. They are not expensive to create, and delaying instantiation helps with getting dialog ownership correct as `MapTool.getFrame()` will have had a chance to be initialized.
|
This is great. I do, however, wish the "Reset Picks" button was inline with the "Pick Once" check box and disabled when the checkbox is unchecked though. |
Collaborator
Author
Sure, I can move it inline. The button is already being disabled as you'd like. |
|
Heh, right after I hit send my brain registered your disabled description in the OP. ;) |
- The panel is now split vertically in two. The top half is a JPanel containing all the table options on a grid, while the bottom half is the scroll pane containing the table data. - Replaced the separate "Table Image" label with a border title on a new panel containing the `tableImage` field. - Fix the size of the `tableImage` field at 150x150. - Reduced the preferred scrollable viewport size of the JTable from 400 to 150 to avoid excessively tall dialogs or strange resizing. This is unfortunately a manually specified value for JTable, so not much opportunity to be smart about it.
This makes everything strongly typed as we don't need to cast the results of looking up String component names.
The new "Pick Once Table" checkbox allows the user to set a table as pick once without resorting to macros. When a table is pick once: - The roll option is disabled as it is not used. - A new "Picked?" columns is shown in the table to indicate whether a raw has already been picked. The user can manually select or unselect these to mark specific entries as picked. - The new "Reset Picks" button will be enabled, allowing the user to clear all the "Picked?" checkboxes. This also means that editing a table will no longer clear the picked entries unless the user manually clears them or changes the table to not be a pick once table.
kwvanderlinde
force-pushed
the
feature/5818-pick-once-ui
branch
from
d76a933 to
8c9a78c
Compare
cwisniew
approved these changes
Nov 18, 2025
github-project-automation
bot
moved this from Awaiting-Review
to To-Be-Merged
in MapTool 1.19
github-merge-queue
bot
removed this pull request from the merge queue due to invalid changes in the merge commit
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Identify the Bug or Feature request
Resolves #5818
Description of the Change
Adds a few elements to the Edit Lookup table dialog to support pick once tables:
When (1) is selected, the Roll field is disabled, (2) is enabled, and (3) is made visible. When (1) is not selected, the Roll field is enabled, (2) is disabled, and (3) is hidden.
As with the existing UI, none of the new element effect the table until the Accept button is hit. Also, since picks are actually handled now, they won't be lost when a table is edited unless the user explicitly resets them.
Also did some refactoring and fixes on
EditLookupTablePanel:GenericDialogfor some nice features.String. This is needed to support the newboolean"Picked?" column.Possible Drawbacks
Should be none
Documentation Notes
The UI can now be used to set a table as pick once and to modify the picked entries.
When pick once is enabled:
When pick once is disabled:
Release Notes
This change is