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

[ticket/12431] Add has_poll icon to topiclists #2340

Merged
merged 2 commits into from May 11, 2014

Conversation

Projects
None yet
3 participants
@PayBas
Copy link
Contributor

commented Apr 22, 2014

@nickvergessen nickvergessen changed the title [ticket/12431] Add has_poll icon to topiclists [WIP][ticket/12431] Add has_poll icon to topiclists Apr 22, 2014

@nickvergessen

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2014

Marked as WIP because the image is missing

PayBas added a commit to PayBas/RecentTopics that referenced this pull request Apr 23, 2014

@nickvergessen

This comment has been minimized.

Copy link
Contributor

commented May 4, 2014

Still missing the image?

@PayBas

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2014

Yeah
On May 4, 2014 4:20 PM, "Joas Schilling" notifications@github.com wrote:

Still missing the image?


Reply to this email directly or view it on GitHubhttps://github.com//pull/2340#issuecomment-42133990
.

Merge pull request #4 from prototech/ticket/12431
[ticket/12431] Add topic poll icon.

@PayBas PayBas changed the title [WIP][ticket/12431] Add has_poll icon to topiclists [ticket/12431] Add has_poll icon to topiclists May 4, 2014

@@ -163,6 +163,7 @@ <h2 class="forum-title"><a href="{U_VIEW_FORUM}">{FORUM_NAME}</a></h2>
</div>
<!-- ENDIF -->
<div class="responsive-hide">
<!-- IF topicrow.S_HAS_POLL -->{POLL_IMG} <!-- ENDIF -->

This comment has been minimized.

Copy link
@nickvergessen

nickvergessen May 4, 2014

Contributor

Should be topicrow.POLL_IMG like for ATTACH_ICON_IMG ?

This comment has been minimized.

Copy link
@nickvergessen

nickvergessen May 4, 2014

Contributor

Also the image does not have a hover text for me?

This comment has been minimized.

Copy link
@PayBas

PayBas May 4, 2014

Author Contributor

1: I adhered to the naming scheme of REPORTED_IMG, DELETED_IMG, etc. because it looks better in the viewforum.php IMG vars assignment. If you want to change it, that's fine too.
2: None of the icons in viewforum rows have this. So you will just get the <dt title="No unread posts"> if set. If we wanted to change this and add title= attributes to all icons, that would be a different topic.

This comment has been minimized.

Copy link
@prototech

prototech May 4, 2014

Contributor

Added title in #2408

This comment has been minimized.

Copy link
@nickvergessen

nickvergessen May 4, 2014

Contributor

@PayBas 1. why is ATTACH_ICON_IMG different? is there a reason for it?

This comment has been minimized.

Copy link
@PayBas

PayBas May 4, 2014

Author Contributor

As far as I can tell, those without ICON_ are assigned only 1 time ("simple icons"), and those with ICON_ are assigned for each topic row (more complex logic, like TOPIC_ICON_IMG). That's why they have topicrow..

If we continue this distinction, POLL_IMG seems to be better. But I agree that the whole thing seems rather confusing.

@@ -748,6 +748,7 @@
1 => '%d private message in total',
2 => '%d private messages in total',
),
'TOPIC_POLL' => 'Topic has a poll',

This comment has been minimized.

Copy link
@nickvergessen

nickvergessen May 8, 2014

Contributor

Should be a full sentence, e.g. This topic has a poll.
Like This topic has been deleted. for deleted and At least one post in this topic has not been approved. for unapproved posts

nickvergessen added a commit to nickvergessen/phpbb that referenced this pull request May 11, 2014

Merge pull request phpbb#2340 from PayBas/ticket/12431
[ticket/12431] Add has_poll icon to topiclists

* PayBas/ticket/12431:
  [ticket/12431] Lang full sentence
  [ticket/12431] Add topic poll icon.
  [ticket/12431] Add has_poll icon to topiclists

@nickvergessen nickvergessen merged commit 1cdff60 into phpbb:develop-ascraeus May 11, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@PayBas PayBas deleted the PayBas:ticket/12431 branch May 12, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.