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

PR: Adds the ability to set a custom ListCellRendered for the popup #49

Closed
wants to merge 5 commits into from

Conversation

lucasecardoso
Copy link

This refers to issue #48

Copy link
Owner

@eugener eugener left a comment

Choose a reason for hiding this comment

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

Overall I think the PR is solid. Please fix those cosmetic API things before I will merge. Also it would be nice if you can submit a demo code which demonstrates the use of the custom renderer as part of this PR


private CheckList(JList list, CheckListRenderer customRenderer) {
if (list == null) throw new NullPointerException();
this.list = list;
Copy link
Owner

Choose a reason for hiding this comment

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

Probably have to check that customRenderer us not null

this.list.getSelectionModel().setSelectionMode(ListSelectionModel.SINGLE_SELECTION);

if ( !isEditorAttached() ) list.addMouseListener(checkBoxEditor);
this.list.setCellRenderer(customRenderer);
Copy link
Owner

Choose a reason for hiding this comment

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

It is better to use setCellRenderer method, since you already have it, instead of duplicating the code.

return renderer;
}

public void setCheckListRenderer(ListCellRenderer renderer) {
Copy link
Owner

Choose a reason for hiding this comment

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

This method is part of fluent interface and not a setter, so to not confuse a user I would remove the word set and leave just checkListRenderer

@lucasecardoso
Copy link
Author

lucasecardoso commented Sep 22, 2016

Sample usage code here

@eugener
Copy link
Owner

eugener commented Sep 22, 2016

There is a simple test code at https://github.com/eugener/oxbow/blob/master/swingbits/src/test/java/org/oxbow/swingbits/table/filter/TableFilterTest.java

Do you think your snippet can be added there?
Sorry I wasn't clear enough about that :)

@lucasecardoso
Copy link
Author

I just realized that the feature gets served better by the useTableRenderers() method. I'm not sure it makes sense to go ahead anymore? If you feel like it's still a good thing to have I'll set up the demo code. Let me know!

@eugener
Copy link
Owner

eugener commented Sep 26, 2016

Closing.

@eugener eugener closed this Sep 26, 2016
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.

2 participants