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

#18 combine name and email display #23

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

florianm
Copy link
Contributor

@florianm florianm commented Jan 6, 2015

As discussed in (messy and closed) PR #22 this PR combines the display of name/email combo fields by hiding the name field, and letting the email field render the email address (if given) with the specified name field (if given). Addresses #18

Review on Reviewable

…ication with their corresponding email fields
@wardi
Copy link
Contributor

wardi commented Jan 8, 2015

we still need the thing that checks for 'display_field': False, right? Alternatively we could use 'display_snippet': null.

@florianm
Copy link
Contributor Author

florianm commented Jan 9, 2015

Strangely, display_field: false just works - although I can't find where that happens, at least there's nothing acting upon display_field or manipulating exclude fields in ckan or scheming as far as I can grep. Any ideas?

@wardi
Copy link
Contributor

wardi commented Jan 19, 2015

Hmm. doesn't seem to hide the fields when I try it.

@florianm
Copy link
Contributor Author

Found why it works on my end - I'm still using my much too clever additional info display snippet with special treatment for author and maintainer fields.

@wardi
Copy link
Contributor

wardi commented Jan 28, 2015

@florianm ok, should we close this too, or will you add the display_snippet is None check to this PR?

@florianm
Copy link
Contributor Author

@wardi sorry I got sidetracked - I will add the check to this PR today. Edit: soon.

@florianm
Copy link
Contributor Author

florianm commented Feb 4, 2015

@wardi added a property "hide_field", formatted additional_info.html for readability and added check for hide_field there, added email example to camel_photos.json.

Up for discussion:

  • The author-hiding property is named hide_field (instead of display_field) so the template can check for its existence and whether to hide the field in one operation.
  • I've added values for the email example to tests which would otherwise fail. I can create a dataset without author and email fields through the GUI, but the tests will fail on blank author/email.

@florianm
Copy link
Contributor Author

@wardi I need to double-check whether dataset notes are hidden inadvertently, sorry that's one thing I might have clobbered last minute!

@florianm
Copy link
Contributor Author

@wardi there's travis errors with the solr server that I'm sure I didn't touch. Any ideas whether that's a problem to do with my changed templates?
Also, please allow me a few days of thinking (my debugging appears to slow down when nappie changing instead of sleeping) to figure out why dataset notes are not showing on my end.

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.

2 participants