Ant's theme updates #689

eurich · 2013-07-17T14:34:54Z

Theme updates from Ant.

…, #670. ================================================= sources/Subs.php Took the forced styling out of the me tag array. People can play with it in the css. Re-wrote the main menu array as per topic at Elk. ------------------------------------------------- sources/controllers/Memberlist.controller.php Removed redundant th classes. Added some that will be needed. ------------------------------------------------- sources/admin/ManageBans.php sources/admin/ManageCalendar.php sources/admin/ManageMail.php sources/admin/ManageMembergroups.php sources/admin/ManageSmileys.php Changed a button class to one of the new classes. Removed some inline CSS. ------------------------------------------------- sources/admin/Modlog.php Removed redundant th classes. ------------------------------------------------- sources/admin/Packages.php Changed a button class to one of the new classes. Removed some inline CSS. ================================================= themes/default/css index.css and index_light.css have had a lot of changes. Too many to list. We haz shiny new theme. :D admin.css had a few smallish tweaks (some temporary, some not). rtl.css and install.css just had some old class names changed to new ones. ------------------------------------------------- themes/default/languages Admin.english.php just had one letter changed (didn't need a captial). index.english.php had some new strings added, some old ones changed, and a bit of re-organising. ------------------------------------------------- themes/default/scripts post.js - Fixed some old markup, to match the new template and css. Added a coment about a stray line break. This could be the cause of a minor bug with SCE and Firefox. Didn't remove it because not sure why it is there. ------------------------------------------------- themes/default Admin.template.php - Minor class updates, and fix for #617 BoardIndex.template.php - Some markup clean up, and moved news fader to index.template.php. Calendar.template.php - New markup and classes stuff. -------- Display.template.php, ModerationCenter.template.php, PersonalMessage.template.php, Profile.template.php, ProfileInfo.template.php, Recent.template.php, Search.template.php - Bug fixes for markup (mostly quickbuttons, some re-ordering for consistency across templates, includes fix for #662 in Display, and for #646 in Recent). -------- GenericMenu.template.php - Fix for #617. index.template.php - Markup revisions for more sanity (very clean now). Addition of news fader. Fix for #640. BadBehavior.template.php, Errors.template.php, GenericControls.template.php, ManageMembergroups.template.php, ManageNews.template.php, ManagePermissions.template.php, Memberlist.template.php, ModerationCenter.template.php, Post.template.php, Who.template.php - Minor class updates. Themes.template.php - Finishing touches for #645 markup. Signed-off-by:Antechinus <darthechidna@bigpond.com>

Signed-off-by:Antechinus <darthechidna@bigpond.com>

norv · 2013-07-17T21:19:30Z

themes/default/Display.template.php

@@ -214,10 +214,11 @@ function template_main()
 		}

 		// Hide likes for those who can't like or unlike for now..
-		if ($message['likes_enabled'])
+		// Really, guests and bots shouldn't get to play with this.
+		if ($message['likes_enabled'] && $context['user']['is_logged'])


Shouldn't this be $message['can_like'], set in core to be false for guests? (I didn't check when is 'likes_enabled' true)

question: should the "liked by ..." indicatior be visible for all members, even those who don't have "can_like" set to true? IMHO yes, that's the reason for the likes_enabled check.

You're right, 'liked_by', for all, and button displayed for who 'can_like'. None of the two visible when the likes feature is disabled at forum level...
Checking it out: that's how it was, and it was perfectly fine.
eurich@fe52ebd#L1R216

I made a PR a few days ago where I tried to put that logic back in sources and not the template ... sorry on the run right now so can't link to it ... the idea was to show the number of likes (assuming a post had been liked) to everyone and only show the like/unlike to those that can use those capabilities. That logic should have been set where the can_like is setup AFAIR

Spuds · 2013-07-17T21:39:57Z

likes enabled should be the master setting ... when its on in the ACP ...
can_like is for the specific user based on permissions, not their post, post count limits, etc etc and should be used for display purposes
May need to check that can_like honors likes_enabled, that would be something I would forget to do :P

norv · 2013-07-17T22:01:46Z

themes/default/css/index.css

-	line-height: 2.0em;
+/* This one is not sprited yet. */
+.linklevel1.likes_button:before {
+	background: url(../images/icons/heart.png) 2px 50% no-repeat;


just to note, we should choose another image too.

just to note, we should choose another image too.

Where's the love ... ah there it is ❤️

StealthWombat · 2013-07-17T22:52:24Z

I think the heart is ok. It's what vB uses for likes, and it's clear and simple. Maybe not quite so red though.

Spriting it up is easy, but I had another idea. Why is the quick edit an inline image anyway? Seems to me we could just do the onclick on the anchor instead of the image, and set the icon as a background image like all the rest. That would save a bit of CSS trickery.

norv · 2013-07-18T03:47:07Z

themes/default/css/index.css

-	background: #eee;
-}
-
+/* Nobody ever uses the silly thing, but anyway.... */


I don't think this qualifies as a helpful comment. Please lets remember that people read and work with this code. Even if it doesn't bring some new info to the code, a comment can be helpful as a stop-gap for readability of the code overall, so IMO even a simple notification 'the openid login box' or so, is not useless. (and it can be improved in time)

Signed-off-by:Thorsten Eurich <thorsten@eurich.de>

norv · 2013-07-18T16:38:26Z

themes/default/languages/index.english.php

@@ -150,6 +188,7 @@
 $txt['there_are_unapproved_topics'] = 'There are %1$s topics and %2$s posts awaiting approval in this board. Click <a href="%3$s">here</a> to view them all.';
 $txt['send_message'] = 'Send message';

+$txt['msg_alert_none'] = 'No messages...';


this little bit (and line 162-163) also overwrite the msg_alert_none change.

Signed-off-by:Thorsten Eurich <thorsten@eurich.de>

norv · 2013-07-19T08:35:10Z

Fixed the likes back to expected display; rebased and merged directly in master. I hope I didn't miss things.

@eurich I rebased a few of the fixes in the first commit, because the commit was reverting things, then the fixes were reverts of the revert. :) This kind of issues may happen, but if they stay that way, they're difficult to read afterwards in history, and make for a cluttered log later.

(A good idea is --amend for the next fix; otherwise it's misleading later, to look at history and think something was missed, when the exactly next one-line commit fixes a trivial miss.)

On a related side note, this is why I'd recommend rebases over merges, too, when updating a branch. It makes a cleaner history, and we can see issues (in a merge commit with conflicts solved it's very difficult, practically unrealistic, to see anything missed).

ETA: I did miss things. Fixing them atm.

norv · 2013-07-19T08:45:15Z

@StealthWombat thank you for the changes. Please consider to make smaller changes to go into one commit, because many unrelated changes are difficult to review and may introduce regressions easily. Also harder to read later.

Please take note that you can also edit the files from the web interface. It's probably so easy to do that you may find it useful for very small fixes (I don't recommend it for anything significant). But much less time goes into directly editing a file, than in posting in a bug the 6-lines fix, then adding the fix to a set of files proposed, then taken by someone and diff-ed and pushed, then reviewed together with 1500+ changes.

ETA: please check the current version. It looks awesome, but some things are broken for me at an immediate test.

@eurich Thank you for the work on this, I'll update the site. :)

norv · 2013-07-19T14:44:28Z

@Spuds That logic is already in the sources, or I'm not sure what you mean?

Spuds · 2013-07-19T14:51:41Z

Sorry for creating extra confusion, I was just trying to clear up what those vars were set to (from the last commit I did) so they were used properly in the template (and I think they are)

StealthWombat · 2013-07-19T21:57:31Z

"Please consider to make smaller changes to go into one commit, because many unrelated changes are difficult to review and may introduce regressions easily. Also harder to read later."

Yeah sorry about that. This one sort of snowballed, because I kept finding related things that needed fixing when I was fixing something else. I'll keep a tighter rein on future ones.

"ETA: please check the current version. It looks awesome, but some things are broken for me at an immediate test."

Will check it and see what the broken bits are.

emanuele45 · 2013-07-19T22:03:07Z

@StealthWombat take also this in consideration #695 ;)

StealthWombat · 2013-07-19T22:39:01Z

Yes I saw that. When Ema plays around with Subs, scary things happen. 👅

Will worry about that one once it is committed to the repo.

Have just loaded the new repo zip into local and done a quick check for broken stuff. From what I can tell so far it's ok, except for a few submit buttons which need their classes changed. There's a lot of basic class cleaning up needed anyway, in various places.

Should probably make the next PR do some of those. The theme itself is mostly sorted, so it'll be down to the cleaning up old crud stage. Once that's done it should all be pretty good.

ETA: Oh bugger. I just loaded the new stuff over my local without backing up, so that's some exta work I've lost. I forgot the "local backup" I made was before I did some more fixing (haven't worked on it for a couple of days). Meh. I hate that.

You're probably seeing things like submit buttons sizing not matching select sizing in some browsers. I had that fixed (really, I did) but just overwrote all that. Will resurrect said fixes soon (browser-specific inconsistencies grumble grumble).

StealthWombat · 2013-07-19T22:51:29Z

ROFLMAO. Ok, the memberlist is broken. TBH I didn't check that stuff Ema did. I just copied over the bits I thought it needed.

There's a problem with the table column counting. It's tripling the custom field columns for some reason. I didn't touch that. It's not set in the template anyway. get Ema to fix it.

ETA: And the new search needs fixing too.

StealthWombat added 2 commits July 16, 2013 17:16

missing language strings..

8bc62fd

Signed-off-by:Antechinus <darthechidna@bigpond.com>

norv reviewed Jul 17, 2013
View reviewed changes

norv reviewed Jul 18, 2013
View reviewed changes

msg_alert_none is now topic_alert_none.

58af93f

Signed-off-by:Thorsten Eurich <thorsten@eurich.de>

norv reviewed Jul 18, 2013
View reviewed changes

eurich added 3 commits July 18, 2013 20:54

Back with some $txt strings... Blame Ant :P

5945179

Signed-off-by:Thorsten Eurich <thorsten@eurich.de>

prettify comment ..

85b2a23

Signed-off-by:Thorsten Eurich <thorsten@eurich.de>

topic_alert_none, the second one..

0e877b0

Signed-off-by:Thorsten Eurich <thorsten@eurich.de>

norv closed this Jul 19, 2013

StealthWombat mentioned this pull request Jul 19, 2013

Broken memberlist #696

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ant's theme updates #689

Ant's theme updates #689

eurich commented Jul 17, 2013

norv Jul 17, 2013

eurich Jul 18, 2013

norv Jul 18, 2013

Spuds Jul 18, 2013

Spuds commented Jul 17, 2013

norv Jul 17, 2013

Spuds Jul 17, 2013

StealthWombat commented Jul 17, 2013

norv Jul 18, 2013

norv Jul 18, 2013

norv commented Jul 19, 2013

norv commented Jul 19, 2013

norv commented Jul 19, 2013

Spuds commented Jul 19, 2013

StealthWombat commented Jul 19, 2013

emanuele45 commented Jul 19, 2013

StealthWombat commented Jul 19, 2013

StealthWombat commented Jul 19, 2013

Ant's theme updates #689

Ant's theme updates #689

Conversation

eurich commented Jul 17, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spuds commented Jul 17, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StealthWombat commented Jul 17, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norv commented Jul 19, 2013

norv commented Jul 19, 2013

norv commented Jul 19, 2013

Spuds commented Jul 19, 2013

StealthWombat commented Jul 19, 2013

emanuele45 commented Jul 19, 2013

StealthWombat commented Jul 19, 2013

StealthWombat commented Jul 19, 2013