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

Refactor Registration Page to use EL #137

Merged
merged 2 commits into from
Nov 16, 2019
Merged

Conversation

G-Pex
Copy link
Collaborator

@G-Pex G-Pex commented Nov 16, 2019

No description provided.

Comment on lines +5 to +15
<c:choose>
<c:when test="${registerStatus == 'success'}">
<t:registersuccess />
</c:when>
<c:when test="${registerStatus == 'fail'}">
<t:registerfail />
</c:when>
<c:otherwise>
<t:registerform />
</c:otherwise>
</c:choose>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah this is dope! Nice work :)

Copy link
Collaborator

@reecebenson reecebenson left a comment

Choose a reason for hiding this comment

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

Nice mate! A few optional changes to make but won't stop a merge. If you want to make the changes, that'll be great otherwise I'm happy for this to be merged! (Do as you please!)

Comment on lines +44 to +45


Copy link
Collaborator

Choose a reason for hiding this comment

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

Unnecessary line breaks, but won't stop a merge.

</div>
</div>
<div class="col s12 center-align">
Already have an account? <a href="login">Login here</a>.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could use ${pageContext.request.contextPath}/login for path consistency :)

<div class="col s12 center-align">
<div class="red-text">
Please ensure you make a note of this password before proceeding to
<a href="login">login</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could use ${pageContext.request.contextPath}/login for path consistency :)

@@ -1,97 +1,10 @@
<%@ page contentType="text/html;charset=UTF-8" language="java" %>
<%@taglib prefix="t" tagdir="/WEB-INF/tags"%>
<%@ taglib uri = "http://java.sun.com/jsp/jstl/core" prefix = "c" %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could remove the spaces here around the attribute/values, etc.

@reecebenson reecebenson changed the title refactor registration to use EL Refactor Registration Page to use EL Nov 16, 2019
@reecebenson reecebenson added this to In progress in ESD Kanban Board via automation Nov 16, 2019
@reecebenson reecebenson added this to the Sprint 2 milestone Nov 16, 2019
@G-Pex G-Pex merged commit 76902f8 into master Nov 16, 2019
ESD Kanban Board automation moved this from In progress to Done Nov 16, 2019
@G-Pex G-Pex deleted the sprint2/refactor-register-jsp2el branch November 16, 2019 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants