profile-menu needs its own division…
-
…in order to be formatted. Without a division, my changes (padding and margins) didn’t take on the list, and instead inherited the properties from the container division (main).
Quite simple…on line 221 of functions.bb-template.php, I just changed:
$list = “<ul id=’profile-menu’>”;
to:
$list = “<div id=’profile-menu’><ul id=’profile-menu’>”;
and, of course, closed it a few lines below.
It’d be nice to have this added to the next revision, so I don’t have to re-insert it.
Thanks…
-
In theory, and really its in theory given the amount of “core” hacks that are needed for any real diversity in theming bbPress, you should add the encapsulating “<div>” tags into your theme instead.
Given that 1.1 (or 1.0.3 – as people use different names for the next release) will be a must upgrade from 1.0.2 if you ask me, i’d hold off on any core hacks for a while if possible.
1.1 (or 1.0.3) includes a good number of bug fixes that have been hanging around for years and years; as well as 2 new features that we already had as plugins.
Either way mZimmers, glad to see you on board and getting your head round it mate!
$list = “<div id=’profile-menu’><ul id=’profile-menu’>”;
seriously?
What’s wrong with it?
Far too little of bbPress’s output comes with relavant IDs or classes for theming; and far too much of it is is hardcoded into the core rather than in the theme, or even a functions file etc.
duplication of ID for a start…
The container having an ID is fine, but inside I’d suggest using classes.
But if all that is in that div is that ul, then the div may not be needed.
Morning Dudes,
duplication of ID for a start…
Maybe you could have said that instead of just typing” seriously? “.
The container having an ID is fine, but inside I’d suggest using classes.
Why? It’ll be unique on the page therefore an ID is by far the better options (quicker use of CSS selectors, much easier for the DOM, and also allows better integration with any JavaScript or Client side CSS changes).
But if all that is in that div is that ul, then the div may not be needed.
Container DIVs (at one level of geneology) are very very useful.
It allows quicker/better DOM iteration, and allows for much more flexible theming especially if dealing with floats, placements (relative/absolute), and really helps with backwards compatibility. I’m all for minimalist design and code, and I know that there is a cry to avoid “divitus” these days, but really having 1 wrapper div to allow you play with margin/padding/width combination really shouldn’t be frowned upon. Especially in software that hardcodes so much into it’s core, and really doesn’t like outputitng IDs or Classes.
Mzimmers, I’d still suggest putting the DIV in your theme instead of the ‘core’, just because I do believe that we’ll be upgrading to a new release relatively soon (i.e. within 2010).
Div’s in themes are fine, div’s in core might not be. Have to agree with you there. Div’s probably belong more in themes.
I suffer from divitus myself, having to put them in in places for other people to use… When coding just for myself I tend to be as minimalistic as possible, if the UL doesn’t need a div around it, then it doesn’t have to be there, then I won’t add it.
If you can guarantee something will always be unique then sure use id’s. However I recently tried to add to that list, found out I couldn’t (for what I was doing) so had to set up a class, and then replicate the CSS (ok I just added the class to where the id was). However if it was a class I could have just added it and used it straight away.
But yeah i should have explained in my first reply, my bad.
All good Rich mate
(am liking your akisnet plugin btw)
Kevin –
Good call on doing it in my theme — that’s actually what I did.
Regarding my method of implementation, my goals were 1) efficacy and 2) minimal coding intrusion. I’m sure there are more elegant alternatives, but I just wanted to fix a problem, and I did.
Fixing the problem with minimal issues is the way to treat bbPress mZimmers. Really glad to see you getting through it bud
- You must be logged in to reply to this topic.