-
Notifications
You must be signed in to change notification settings - Fork 184
Read-only addressbook improvements #299
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #299 +/- ##
=======================================
Coverage 14.38% 14.38%
=======================================
Files 55 55
Lines 1251 1251
=======================================
Hits 180 180
Misses 1071 1071 Continue to review full report at Codecov.
|
5b2c66c
to
4a9349c
Compare
Hum, 1d1ccda broke the import |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 'Detailed name' is editable for me in read-only view
- 'Gender' is not displayed in read-only mode
Edit: The only thing that bugs me is the two sets of templates...
Fixed! Don't know why I missed these! ¯_(ツ)_/¯
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
1a43cb9
to
ff5e29b
Compare
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
I'm a bit late to the party, but why did you create a separate readonly template for every existing template? Something like: <input [...] ng-disabled="ctrl.addressBook.readOnly"> |
Tried that already, didn't liked the idea since a user can manually edit the field to remove the disabled state. But yes, I can push those commit if you prefer. I kind of like not having two templates too. :) |
ff5e29b
to
b9e0f2e
Compare
@Henni @irgendwie Done! Switch to previous commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @skjnldsv 👍
Uh oh!
There was an error while loading. Please reload this page.