8375573: JTable ignores setPreferredWidth during initial layout when AUTO_RESIZE_LAST_COLUMN is enabled - #29291
8375573: JTable ignores setPreferredWidth during initial layout when AUTO_RESIZE_LAST_COLUMN is enabled#29291prsadhuk wants to merge 4 commits into
Conversation
…AUTO_RESIZE_LAST_COLUMN is enabled
…AUTO_RESIZE_LAST_COLUMN is enabled
|
👋 Welcome back psadhukhan! A progress list of the required criteria for merging this PR into |
|
@prsadhuk This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 148 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
| // is set to last column on account of | ||
| // AUTO_RESIZE_LAST_COLUMN autoResizeMode | ||
| for (int i = 0; i < columnModel.getColumnCount(); i++) { | ||
| if (columnModel.getColumn(i).getPreferredWidth() != 75 |
There was a problem hiding this comment.
Why are we using hardcoded value here ? Any particular reason for checking width against value 75 ?
There was a problem hiding this comment.
Because default width is such and there is no way to ascertain default width..
jdk/src/java.desktop/share/classes/javax/swing/table/TableColumn.java
Lines 523 to 524 in b5727d2
jdk/src/java.desktop/share/classes/javax/swing/table/TableColumn.java
Lines 556 to 557 in b5727d2
There was a problem hiding this comment.
Got it, it's been set here -
| for (int i = 0; i < columnModel.getColumnCount(); i++) { | ||
| if (columnModel.getColumn(i).getPreferredWidth() != 75 | ||
| && columnModel.getColumn(i).getWidth() == 75) { | ||
| prefWidthSet = true; |
There was a problem hiding this comment.
Can break the loop for first true condition instead of iterating through all the columns ?
There was a problem hiding this comment.
Yes, can do..Updated..
| " initial layout when AUTO_RESIZE_LAST_COLUMN is enabled"); | ||
| } | ||
| } finally { | ||
| if (frame != null) { |
There was a problem hiding this comment.
Normally we do this inside EDT.
There was a problem hiding this comment.
its already under EDT
There was a problem hiding this comment.
Ok, I see this is in EDT. But now this one will throw InvocationTargetException along with RuntimeException ? I guess it's better to move out the exception handling outside EDT.
There was a problem hiding this comment.
Dont think it matters..There are many tests that follows this..throwing exception inside EDT..
additionally it will result in volatile usage of columnCount/width which we can avoid here..
|
/integrate |
|
Going to push as commit 55375e9.
Your commit was automatically rebased without conflicts. |
AUTO_RESIZE_LAST_COLUMN handling was done in JDK-8234071 but it didn't honour the preferred width if it is already been set for each column so when layouting is done, it only sees the
resizingColumnis set to AUTO_RESIZE_LAST_COLUMN and adjust only the last column and distribute width equally to all other column.
Fix is made to honour the user-set preferred column width even if auto-resize mode is set to LAST_COLUMN
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/29291/head:pull/29291$ git checkout pull/29291Update a local copy of the PR:
$ git checkout pull/29291$ git pull https://git.openjdk.org/jdk.git pull/29291/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 29291View PR using the GUI difftool:
$ git pr show -t 29291Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/29291.diff
Using Webrev
Link to Webrev Comment