Skip to content
This repository has been archived by the owner on Dec 22, 2023. It is now read-only.

Students have phones too #40

Merged
merged 10 commits into from
Aug 20, 2015
Merged

Students have phones too #40

merged 10 commits into from
Aug 20, 2015

Conversation

vertein
Copy link
Contributor

@vertein vertein commented Aug 18, 2015

Screenshots

New:
Clean State:
image

Stacks on screen size small now:

image

Looks like someone should add a phone number:
image

Looks like you need a type before clicking that save button:
image

State with information entered:
image

And you can always hit update and go back and update (but not delete):

image

Loading state:
screenshot - 08192015 - 04 11 40 pm

JSONObject json = ContactInformationMapper.convertToJSONObject(emergencyContacts, localInfo, phNumberWithDate);
json.put("NETID", netId);
if(logger.isTraceEnabled()) {
logger.trace("Saving the following JSON: " + json.toString());
Copy link
Member

Choose a reason for hiding this comment

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

slf4j enables a not-if-gated logger.trace("Saving the following JSON: {} .", json); at no meaningful performance penalty vs this syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take a look. This was copy and pasted, so I might look at our other uses of this.

@timlevett
Copy link
Collaborator

No additional suggestions from me.

@vertein vertein force-pushed the studentsHavePhonesToo branch 2 times, most recently from 0b58e70 to 069b7c0 Compare August 19, 2015 19:55
@apetro
Copy link
Member

apetro commented Aug 20, 2015

👍 This is progress, it fulfills the story, I'm entirely in favor. I do predict that we'll eventually circle back to allow multiple phone numbers, but let's ship this excellent progress and see what the reaction is.

@keirserrie
Copy link

👍

vertein added a commit that referenced this pull request Aug 20, 2015
@vertein vertein merged commit 76b52f1 into master Aug 20, 2015
@vertein vertein deleted the studentsHavePhonesToo branch August 20, 2015 18:52
@jhanstra
Copy link
Contributor

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants