Wedge

Public area => The Pub => Features => Topic started by: Nao on October 19th, 2011, 10:15 AM

Title: Moving options out of Current Theme
Post by: Nao on October 19th, 2011, 10:15 AM
(split from New Rev comments)

Found the origin of the bug. Comment out the $current_action = 'moderate' line in Subs.php and it doesn't generate the error anymore... I think it's related to how I moved the moderation center to within the admin menu. Didn't look any further.
Posted: October 19th, 2011, 09:43 AM

I'm seeing FOUS effects in the admin area when dealing with the error log... Hadn't seen these in a while. Dunno if it's due to some bug in the new Opera. Can you tell me if you've seen any of these in Chrome?
Quote from Arantor on October 19th, 2011, 03:17 AM
It's to make it easier for themes to add options, that are under admin control. There's no way for themes to add user options AFAIK without replacing some/all of the profile template.
There are plenty of options that should be in general options...

Fading delay between items for the news fader: NEWS area
Number of recent posts to display on board index: possibly elsewhere. That's in the info center right..? See below.

Show statistics on board index: --> I don't believe themes would modify the info center so much that this would kill layout...

Show latest member on board index: ditto (might be removed entirely)
Show group key on board index: ditto (should be removed as an option, and always shown.)

"Show who is viewing" --> this is an important feature, it should be enabled by default at the very least... (And the English description is confusing, too.)

Show last modification date on modified posts: if themes don't provide a field for that, then it just won't be shown... Otherwise, they'll provide for it, and make sure it's usable. This is definitely an option for POST settings.
 
Show view profile button under post: POST settings.
Show user avatars in message view: POST settings or AVATAR settings.
Show personal text in message view: POST settings.
Show gender images in message view: POST settings.
 
Hide post group titles for grouped members: POST settings or MEMBERGROUP settings.

Enable collapsible additional post options: general, post settings or something...

My conclusion: theme settings should only *contain settings that related to the theme itself (e.g. something like, "show the info center in the sidebar/main area"), not settings related to features that are built into Wedge* (e.g. personal_text has a field in the members table, so all themes are expected to be able to deal with it.)

What do you think...? As an added bonus, we get to have the options made searchable... This is all because yesterday I wasted 5 minutes trying to enable 'show group key' and the admin search refused to point me to it...

We should also rename 'Current Theme' to 'Current theme settings & options' or something. Yeah, I know, it's a bit too long...
Quote
Admin > Current Theme makes some modicum of sense, but not a lot - though it's a world better than how unobvious theme settings are to find normally.
(It's interesting that Opera is telling me 'unobvious' is not a valid word... I always thought it was. A dictionary gives me these antonyms for obvious: ambiguous, indefinite, obscure, unclear, vague. Hmm, that's very close to what we'd use in French -- ambigu, indéfini, obscur, flou/pas clair, vague.)

If they're actual theme settings, I understand they need to be together in one place, but if they're not logically theme settings per se, they should be elsewhere. At worst, we could add 'aliases/shortcuts' to the correct setting pages in a main page if needed.
Quote
It's a theme option because not all themes provide it.
It's inside the info center... Especially now that InfoCenter is its own template, I don't see how/why themes should delete the option. Additionally -- it's up to the theme to decide what to do. For instance, if I had an admin setting to choose the default sidebar position (left/right), I'd expect the theme to either follow it, or just don't show a sidebar on the left or right (e.g. show it at the bottom). It's a bad example but I hope you get my point...
Quote
Seriously tempted to move that back out into being a plugin anyway because partly not all themes provide it and partly because so many people want it 'ordered how they want, not just alphabetic', and I think it would be a stretch too far to make core with that in mind. It's not like it's tough to expand any more or anything.
I don't know... :-/
I suppose you're right in that sense. Like, people might want to sort membergroups by color brightness... :lol:
Quote
Nice. I spent the rest of my evening trying to score the one remaining achievement I had for Monkey Island II on Steam (complete the game in under 3 hours, just finished with 30 mins to spare), so I haven't looked at the commit yet.
I hope you used a walkthrough for that :P It's easier to get below 3 hours that way.
Don't have Steam though... I only have MI2 on my iPod and I don't think it has achievement lists :P Arrr... How appropriate anyway, I play like a cow!
Quote
Quote
Uncaching on updateMemberData() means doing it on many, many user actions... Might as well not cache anything
Is there really a lot of places that call updateMemberData?
Yeah, pretty much... Over 60 times. And the cache would be updated even if 'id_group/additional_groups' isn't updated, which isn't too great IMNSHO.
Quote
I'll update it if it becomes necessary but given how there haven't been updates for a while and it's one of those things that generally 'just works', it should be OK to fix up now.
Sure. Do you want me to get started on it maybe...? Or would you rather do it? It's not exciting work :P

Heck, technically I thought you were planning to have a minified version of the file... (Maybe we could move Class-SFTP.php to the wip folder, and include the minified version in Sources. No one has to know about the problems it had :P)
Quote
Hmm, we should probably test that. It's likely I'm worrying about nothing but I'd rather that and be proven wrong than to be bitten by it later.
Sure, but I'd rather know why SMF/Wedge is using \n when it could be using br in most places :P
Quote
Quote
In that case, hmmm... How do we do that? Just a simple if (isset($_GET['xml'])) will do...?
Testing for $_GET['xml'] is good but it doesn't automagically do that for the feeds IIRC. But if we had the feeds set that too (if they don't already), then it's a no-brainer.
I'd rather not have ';xml' on any URL that is likely to be reused by someone, reposted on a forum, etc. We can just add a special || action == 'feed' anyway ;)
Title: Re: Moving options out of Current Theme
Post by: Arantor on October 19th, 2011, 11:28 AM
Quote
I'm seeing FOUS effects in the admin area when dealing with the error log... Hadn't seen these in a while.
I see the error log quite heavily, especially at the moment where I'm debugging and have a fairly bad habit of adding debugging logs through trigger_error... but no FOUS issues.
Quote
...info center options...
Yes, recent posts probably should be a general, not a theme, option. If nothing else, a theme which didn't support it before will end up possibly running a modestly expensive query which it didn't have to. The counter-argument is that a theme might have done something very cool and imaginative here but honestly, I'm not seeing what...
Quote
Show statistics on board index:
Given that it's now a single block in the layout I'd have to argue that if they did do something that caused it to break, they're doing it wrong.
Quote
Show latest member on board index
I have yet to fully understand what the point of this actually is. The only reason I don't disable it in my installs is that it doesn't generate any extra load because it's still calculated whether it's shown or not, and it defaults to on. Personally I'd be happy with not only removing the option, but removing this in its entirety.
Quote
Show group key on board index
Hmm, this one is tricky. On the one hand, it feels almost it like it should be a plugin and on the other, do we really want/need another query on the board index by default? That's the reason it is actually off by default, to save a query.

Mind you, it's not like we can't cache that. Thing is, though, if it's left in by default it really needs more options, like being able to control which group(s) are shown and in what order.
Quote
"Show who is viewing"
Well, remember this used to be integrated directly inline in the message index and display templates, and I moved them into the sidebar in the relevant places. Consequently it's far less a theme option than it used to be.
Quote
Show last modification date on modified posts
Yup, I can see it's not really a theme option. On the other hand, if someone makes a theme without it in, there's likely going to be a 'this option doesn't work' bug report come in, though it should be possible to see pretty quickly that it's a theme issue.
Quote
Show view profile button under post: POST settings.
Don't we have a user menu for that sort of thing anyway?
Quote
Show user avatars in message view: POST settings or AVATAR settings.
Avatar settings.
Quote
Show personal text in message view: POST settings.
Hmm. I'd say the post bit is one of the most likely things to be changed in a theme (after the overall style index.template and the board index), and whereas I can't realistically see a theme removing the avatar, I can see them removing the personal text.

I'd actually be inclined to say that it should be on by default and if the admin doesn't want to show it, they'll probably disable it anyway in profile fields.
Quote
Show gender images in message view: POST settings.
Pretty much same as above.
Quote
Hide post group titles for grouped members: POST settings or MEMBERGROUP settings.
Membergroup settings, seeing how it has to be handled in the core logic.
Quote
Enable collapsible additional post options: general, post settings or something...
Post settings.
Quote
My conclusion: theme settings should only *contain settings that related to the theme itself (e.g. something like, "show the info center in the sidebar/main area"), not settings related to features that are built into Wedge* (e.g. personal_text has a field in the members table, so all themes are expected to be able to deal with it.)
My take on it is that if it's something that a theme is *typically likely to modify* then it should be handled in the theme as an option. You'll notice that most of the above fall into the 'not likely to modify' category and thus should be general not theme options, IMNSHO.

And generally, yes, if the core provides it, the theme should either suck it up and deal with it, or if it doesn't want to provide for it, it should be pretty clear that it's the theme making the decision.
Quote
(It's interesting that Opera is telling me 'unobvious' is not a valid word... I always thought it was. A dictionary gives me these antonyms for obvious: ambiguous, indefinite, obscure, unclear, vague. Hmm, that's very close to what we'd use in French -- ambigu, indéfini, obscur, flou/pas clair, vague.)
It isn't a valid word. Neither is inobvious.[1]
Quote
It's inside the info center... Especially now that InfoCenter is its own template, I don't see how/why themes should delete the option. Additionally -- it's up to the theme to decide what to do.
Therein lies the point: if the theme doesn't support it, it should remove the option for it.
Quote
I suppose you're right in that sense. Like, people might want to sort membergroups by color brightness...
Sorting by average of RGB values, or converting to HSL and sorting by luminescence? :P Seriously though, I can imagine (and have seen it done many times) where people create groups that aren't alphabetically ordered but have an intrinsic order by precedence. E.g. imagine sm.org. Ordering groups by alphabetic, puts Beta Tester first then Charter Member then Customizer. Presumably you'd want the team first, followed by Charter Member, Friend, Beta Tester in some semblance of order, with the point to prioritise importance.
Quote
What do you think...? As an added bonus, we get to have the options made searchable... This is all because yesterday I wasted 5 minutes trying to enable 'show group key' and the admin search refused to point me to it...
There are so many areas in admin that are not properly covered, it's unreal.
Quote
We should also rename 'Current Theme' to 'Current theme settings & options' or something. Yeah, I know, it's a bit too long...
"Current theme's settings"
Quote
I hope you used a walkthrough for that  It's easier to get below 3 hours that way.
Nope. I didn't exactly hurry, even stopping to listen to the dev commentary in places I didn't remember listening to it before, and still clocked in with half an hour to spare.[2][3]
Quote
Yeah, pretty much... Over 60 times. And the cache would be updated even if 'id_group/additional_groups' isn't updated, which isn't too great IMNSHO.
Eh, didn't realise it was that heavy.
Quote
Sure. Do you want me to get started on it maybe...? Or would you rather do it? It's not exciting work
Nah, I'll do it. I'll probably have to fix stuff. As far as minification goes, I can but it seems like extra work that isn't necessary - getting it down to 280KB was enough of a result for me personally.
Quote
Sure, but I'd rather know why SMF/Wedge is using \n when it could be using br in most places
The main place *I* use it when I do that is for things that will ultimately become emails, since I don't really want to have to think about stripping the br's from it...
Quote
I'd rather not have ';xml' on any URL that is likely to be reused by someone, reposted on a forum, etc. We can just add a special || action == 'feed' anyway
No..... just inside the feed code, add $_GET['xml'] = true :P
 1. However, despite the fact I'm well aware that it isn't a 'real' word in the dictionary sense, does it matter if it conveys the meaning that I'm trying to communciate? It doesn't matter that the word isn't in a dictionary if you and I both understand the same thing from it. Un- is a typical prefix to indication absence or in some cases negation, so un-obvious is a fairly clear, IMHO, meaning of something that has an absence/lack of obviousness, or is simply 'not obvious'. Best thing I learned from David & Leigh Eddings' "The Redemption of Althalus", where one of the characters came out with 'funner', and was appropriately rebuked for using a made-up word, until he pointed out that if you knew what he meant, it didn't matter that it was made up.
 2. I got a couple of things 'wrong' as in slower than I could have done, and the ending always takes much longer than it needs to due to being zapped here, there and everywhere. But even on my first run-through in years, when I first got it on Steam, I still did that in about 3h 15. If I played more carefully, could probably even do it in around 2 hours.
 3. Also note that I've played the LucasArts games pretty heavily over the years and have walkthroughs for most of them in my brain.
Title: Re: Moving options out of Current Theme
Post by: Arantor on February 8th, 2012, 11:15 PM
Bumpity for justice.

* Show recent posts on board index: It's a plugin.

* "Show who is viewing": Features and Options (with the other who's online options)

* Enable collapsible additional post options: general, post settings or something...: Now in Post options.


Everything else is mostly as-is with the following changes:

* Show who edited a post
- Given how many people ask about this, I'm tempted to make it a bit more smart. Right now, I seem to recall it doesn't display the name if the person editing was the author of the post. Now, my thoughts are making it multi-option (and general, not per-theme):
  - Never show who modified a post
  - Always show who modified a post
  - Never show who modified a post if it was the author
Maybe there's more options in there too?

* Group key
Well, it's now displayed in two places, the sidebar and the front page (i.e. both places the info center is), but I'm thinking the sidebar version is badly formatted (and that won't change much), while the front page version... it probably should go into a plugin really. Either that or it needs to be strongly upgraded in core, and I don't feel a burning need to do that.