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

Add properties to the book model #57

Merged
merged 3 commits into from
Nov 28, 2017
Merged

Add properties to the book model #57

merged 3 commits into from
Nov 28, 2017

Conversation

bdr99
Copy link
Contributor

@bdr99 bdr99 commented Nov 7, 2017

These properties are for use on the search results page. The author property is parsed from the YAML using PyYAML. The title_short and description_short properties are generated dynamically.

@bdr99 bdr99 changed the title add author property to the book model Add properties to the book model Nov 7, 2017
@bdr99 bdr99 mentioned this pull request Nov 21, 2017
@bdr99
Copy link
Contributor Author

bdr99 commented Nov 21, 2017

After some investigation, we determined that the CSS approach to truncating text is only possible for single line text. Since we require multiple lines to be displayed, we will have to stay with the server-side truncation.

@eshellman
Copy link
Contributor

for multiline blocks you can use overflow: scroll;

@bdr99
Copy link
Contributor Author

bdr99 commented Nov 21, 2017

@eshellman We're not too sure if that would look better. Do you think scroll bars are the best way to truncate descriptions?

image

@eshellman
Copy link
Contributor

maybe looks better on the mac; if you fix the width the bottom scroll goes away.

@bdr99
Copy link
Contributor Author

bdr99 commented Nov 21, 2017

@eshellman I have made this change as part of #58.

@eshellman
Copy link
Contributor

what's with the travis build failing?

@bdr99
Copy link
Contributor Author

bdr99 commented Nov 28, 2017

@eshellman I'm not sure why it's failing, it seems that for some reason it can't find the .travis.yml file. It's strange because the PR that added that has been merged into master.

@eshellman
Copy link
Contributor

should have realized that a pull from master was needed

@eshellman eshellman merged commit 821a5ee into master Nov 28, 2017
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.

None yet

2 participants