Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve AssetPanelItem #420

Merged
merged 4 commits into from
Aug 29, 2021

Conversation

jdeblander
Copy link
Contributor

@jdeblander jdeblander commented Aug 28, 2021

  • Aligned asset name to center
BEFORE AFTER
image image
  • Made sure the buttonPanel is always the same size
    ( see Cosmetic improvments to the AssetPanelItem #419 )

  • When asset name is to large, the buttonPanel will be shown on top
    image
    image

  • Made clicking the AssetPanelItem easier
    When clicking an asset, I had the issue that one out of three clicks didn't seem to register. The cause of this was a slight movement of my pointer during the click. Therefore I added the mouseRelease as well, beside the mouseClick, so it registers more actions.

Any feedback/remarks welcome.

- Aligned asset name to center
- Made sure the buttonPanel is always the same size
- When asset name is to large, the buttonPanel will be shown on top
- Made clicking the AssetPanelItem easier
Copy link
Member

@nightm4re94 nightm4re94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
I agree with all changes except for the text alignment (mentioned in the suggestion above).

textField.setLineWrap(true);
textField.setColumns(10);
SimpleAttributeSet attributes = new SimpleAttributeSet();
StyleConstants.setAlignment(attributes, StyleConstants.ALIGN_CENTER);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
StyleConstants.setAlignment(attributes, StyleConstants.ALIGN_CENTER);
StyleConstants.setAlignment(attributes, StyleConstants.ALIGN_JUSTIFIED);

I'd prefer justified alignment over centered text. The last line of multiline text should not be centered in my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using justify will more or less look exaclty the same as the not aligned text before.
I'll remove the alignment and go back to a JTextArea, which is easier to use without the alignment styling.
I'll keep the 2 lines and button size changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't center the text if it is a single line, which is still desirable in my opinion. Though I generally dislike the idea of using a JTextPane to display the filename as it is quite heavy. Maybe we can use a JLabel with html content and have the rename action sit inside a context menu with a separate popup?

Copy link
Contributor Author

@jdeblander jdeblander Aug 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the alignment back to before (left) and went back to a JTextArea instead of the JTextPane.
image
To keep the longer names more visible, I kept the following behaviour where a long asset name is using the full available height until the asset is selected. Then the buttons are superimposed.
image
image

Removed unused imports
Removed the center alignment and returned to JTextArea instead of JTextPane
Ungrouped imports
@nightm4re94 nightm4re94 merged commit efa90f5 into gurkenlabs:master Aug 29, 2021
@jdeblander jdeblander deleted the improve-asset-panel-item branch August 29, 2021 17:22
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.

None yet

3 participants