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

Few fixes and features #596

Open
wants to merge 12 commits into
base: rails4
Choose a base branch
from

Conversation

louiecaulfield
Copy link

This pull request fixes a few things. I suppose styles could be broken since I changed the views a bit to reduce code redundancy.

I believe I fixed issues #561, #443, #592 (already marked closed though) and #569

Besides that, a few unreported issues were fixed by commits

  • 1145333 (forum.last_visible_post returned hidden posts)
  • cd39afe (show member post count and member sice)

It's my first contribution to a rails engine, so I'm pretty sure I'm not doing it the right way. Please let me know what needs to change to allow my code to be pulled in.

@radar
Copy link
Collaborator

radar commented Oct 19, 2014

Hi @louiecaulfield, thanks for the PR! Looks like I will need to set aside some time soon to go through and look this over. Would you mind rebasing it on master for me? Thanks.

<td class="topics-count"><%= topics_count(forum) %></td>
<td class="posts-count"><%= posts_count(forum) %></td>
<td class="views-count"><%= forum.views_count %></td>
<td class="topics-count show-for-medium-up"><%= topics_count(forum) %></td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this attribute do?

Copy link
Author

Choose a reason for hiding this comment

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

It's a foundation-zurb thing. I didn't intend this to end up in the pull request. I'll rebase on your master and move site-specific change into a separate branch.

Copy link
Author

Choose a reason for hiding this comment

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

But since you're asking, what it does is it hides the topics count columns on small screens so the forum overview is somewhat functional. Try https://www.lab-nation.com/forum and resize your browser window really narrow.

@louiecaulfield
Copy link
Author

I rebase/merged my rails4 branch on yours and isolated my CSS requirements in a different branch. Should merge fine now.

@radar radar mentioned this pull request Jul 14, 2015
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