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

Fix draft title issue #21

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

Conversation

leninhasda
Copy link

also added "." in css link to make it relative

@torifat torifat assigned mugli and sarim and unassigned mugli Nov 7, 2014
@sarim
Copy link
Collaborator

sarim commented Nov 9, 2014

Thanks for the fixes. But I think css ellipsis will look better here. What do you think?

@torifat
Copy link
Owner

torifat commented Nov 9, 2014

@sarim 👍 I agree

@leninhasda
Copy link
Author

I didn't know there was a css property for that!! :O
BTW if css can do the trick why do we need that title length checking? we can just send full text as title and let css do its job.

Let me know if i am missing or overthinking something. Thank you @sarim vai :)

@sarim
Copy link
Collaborator

sarim commented Nov 9, 2014

Yes, that will work. But will that increase DOM memory consumption or shit like that? I don't know :| Maybe we can chop it to like 100 chars then use css ellipsis just to be sure.

@torifat
Copy link
Owner

torifat commented Nov 9, 2014

Don't worry about memory for a mere title man 😄

@sarim
Copy link
Collaborator

sarim commented Nov 9, 2014

Here title = content if we do that, imagine someone writes an essay :v

@leninhasda
Copy link
Author

hahaha... :v i was just trying to imagine that :P
ok, i am adjusting it to 100 character then.
but may be for my version i will be making option to add custom title :D

@leninhasda
Copy link
Author

actually don't even need to check for 100 characters. its already cutting out first line of the content which i think should do just fine

@torifat
Copy link
Owner

torifat commented Nov 10, 2014

@sarim title ≠ content 😛

@sarim
Copy link
Collaborator

sarim commented Nov 10, 2014

@leninhasda I think this will be better.

if (!!content && content.trim()) {
  return content.substr(0, titleLength);
}

Also can you change href="./css/main.css" to href="css/main.css" ? :)

@leninhasda
Copy link
Author

updated @sarim vai

@sarim
Copy link
Collaborator

sarim commented Nov 11, 2014

one last fix :P please add && content.trim() logic like my previous comment. Without it, if someone types only spaces, draft title becomes empty.

@torifat
Copy link
Owner

torifat commented Nov 11, 2014

@sarim

if (!!content && content.trim()) {
  return content.substr(0, titleLength); // Here content is not trimmed :)
}

@sarim
Copy link
Collaborator

sarim commented Nov 11, 2014

Yes i noticed that, but spaces won't be space when rendered in html :P

@leninhasda
Copy link
Author

ahh.. should have noticed that, stupid mistake on my end :(

i have a question, i can't seem to figure it out which gulp plugin is concating js files and also renaming (both js & css) files to "app"? gulp-rev is adding the hash part, but what about "app" part?

@torifat
Copy link
Owner

torifat commented Nov 12, 2014

@leninhasda uglify is doing the minification & concatenation 😄

@leninhasda
Copy link
Author

i thought uglify only minifies js files & so i use gulp-concate for concatenation :-|

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.

4 participants