Simplify and expand use of LinkBuilder factory methods #251
Conversation
| { | ||
| DetailsURL url = DetailsURL.fromString("/mgap/genomeBrowser.view?database=" + jbrowseId + "&activeTracks=" + trackName, ContainerManager.getForId(containerId)); | ||
| out.write(PageFlowUtil.link("View In Genome Browser").addClass("labkey-text-link").href(url.getActionURL())); | ||
| out.write(LinkBuilder.labkeyLink("View In Genome Browser", url.getActionURL()).addClass("labkey-text-link")); |
There was a problem hiding this comment.
addClass not needed, right (and maybe not needed before either)?
| Container requestContainer = MccManager.get().getMCCRequestContainer(ctx.getContainer()); | ||
| DetailsURL url = DetailsURL.fromString("/mcc/requestReview.view?requestId=" + requestId + "&mode=rabReview", requestContainer); | ||
| out.write(PageFlowUtil.link("Enter Review").href(url.getActionURL().addReturnUrl(ctx.getViewContext().getActionURL())).addClass("labkey-text-link")); | ||
| out.write(LinkBuilder.labkeyLink("Enter Review", url.getActionURL().addReturnUrl(ctx.getViewContext().getActionURL())).addClass("labkey-text-link")); |
There was a problem hiding this comment.
same as above. this was pre-existing your change and doesnt really harm anything either
There was a problem hiding this comment.
Correct... not needed (it's set by default) but harmless. I'll clear these out before merging, just for clarity.
| DetailsURL url = DetailsURL.fromString("/mcc/requestReview.view?requestId=" + requestId + "&mode=primaryReview", requestContainer); | ||
| out.write(HtmlString.BR); | ||
| out.write(PageFlowUtil.link("Enter MCC Internal Review").addClass("labkey-text-link").href(url.getActionURL().addReturnUrl(ctx.getViewContext().getActionURL()))); | ||
| out.write(LinkBuilder.labkeyLink("Enter MCC Internal Review", url.getActionURL().addReturnUrl(ctx.getViewContext().getActionURL())).addClass("labkey-text-link")); |
bbimber
left a comment
bbimber
left a comment
There was a problem hiding this comment.
This pre-dated your change, but most of the edited lines dont need addClass('labkey-text-link'), right? While it would make the code clearer, I dont think it's your responsibility in the merge since it all pre-dated it. Thanks for the refactor...
Correct. Just removed them all, 497f4a6. Because why not. |
Rationale
Using
LinkBuilderfactory methods simplifies the creation of links and provides consistency & clarityRelated Pull Requests