Skip to content

Improve display element builders #6558

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

Merged
merged 3 commits into from
Apr 13, 2025
Merged

Improve display element builders #6558

merged 3 commits into from
Apr 13, 2025

Conversation

labkey-adam
Copy link
Contributor

@labkey-adam labkey-adam commented Apr 11, 2025

Rationale

Making our builders a bit easier to use:

  • Code usually interacts directly with builders, so make the builders top-level classes for import convenience and make the display elements inner classes.
  • Move all builder classes to the same package (org.labkey.api.util)
  • Introduce LinkBuilder static factory methods simpleLink() and labkeyLink() for convenience
  • Introduce InputBuilder static factory methods checkbox(), file(), hidden(), radio(), and text() for convenience
  • Introduce JspBase.simpleLink()

@labkey-adam
Copy link
Contributor Author

@bbimber see LinkBuilder.simpleLink() and other convenience methods introduced here. Better late than never...

@bbimber
Copy link
Collaborator

bbimber commented Apr 12, 2025

@bbimber see LinkBuilder.simpleLink() and other convenience methods introduced here. Better late than never...

Thank you! I think I had my fill of link refactoring for the time being, but am very glad to see it added. I'll also try to break my habit of over-using PageFlowUtil.

Copy link
Contributor

@labkey-jeckels labkey-jeckels left a comment

Choose a reason for hiding this comment

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

Maybe we'll need a simpleLink(String, String) for some of my suggestions in other PRs. Probably worth it to get rid of more clearClasses()

<%@ page extends="org.labkey.api.jsp.JspTest.DRT" %>

<%!
@Test
public void test1()
{
assertTrue(1==1);
ModuleLoader.getInstance().getModules().stream()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intended for this PR? System.out.println()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, unintentional. That snuck out of my "ignore" change list. Reverted.

@labkey-adam labkey-adam merged commit c920720 into develop Apr 13, 2025
2 of 3 checks passed
@labkey-adam labkey-adam deleted the fb_builders branch April 13, 2025 20:24
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.

3 participants