Needs work
Project:
Skinr
Version:
8.x-2.x-dev
Component:
Miscellaneous
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
8 Jul 2010 at 13:58 UTC
Updated:
14 Sep 2015 at 16:37 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
stephthegeek commentedOne option is to just use the page-level styles for this, targeted appropriately in your CSS. But that doesn't help so much if you're rendering that node in different situations, and not just talking about the full page version.
Comment #2
moonray commentedYou can easily write a skinr plugin to take care of this. See the documentation; it's very easy if you have a bit of PHP and Drupal knowledge.
Comment #3
nomonstersinme commentedobviously with theming knowledge you can figure out how to style an individual node to your liking... but i think this would be a useful feature in skinr.
If i knew enough about php i would try and figure this out :/
anyone wanna take a stab?
Comment #4
k3vin_nl commentedOf course it is very easy to style an individual node, but (for us) the (great) point of Skinr is to make theming more sustainable and not dependant upon (volatile) node-id's, page titles, view-names, etc....
With individual Skinr styles for nodes, it would be very easy to set up some predefinied styles that the author of the node can choose from.
Otherwise you could only styles nodes after they have been created and this kind of defeats the purpose of a cms.
The Skinr settings for nodetypes are not very useful to us, because it is already fairly easy to style by nodetype. The only advantage I can see is to be able to style a newly created nodetype based upon a existing Skinr style.
Comment #5
bkosborneI was sent here from IRC.... i'm looking for this as well. It would be great to be able to select a theme to use for a node, as we can for blocks with skinr.... maybe ill take a look at it this weekend
Comment #6
jacineThis isn't something I would probably use, but I don't see how it would hurt.
It's not terribly useful outside of contrib themes unless you are using them with exported content types, where you can easily style against what you know is going to be there. But it does allow easy adding of custom classes and if you have styled your default nodes, and you install Ubercart, you can use Skinr to apply a predefined Ubercart style to the product node type. Not the best example, but hopefully you get what I'm trying to say. There are other things you can do, but it's really what you make of it, IMO.
This is likely not going to make it into 2.x though, so I'll postpone for now.
This is totally unrelated to Skinr. We will not be adding this type of functionality, sorry.
Comment #7
ChrisBryant commented@bkosborne I had thought you wanted to select a skinr skin from a drop down at a per node level, rather than selecting a "theme." As Jacine mentions, Skinr likely won't ever let you choose some entire other theme at a per node basis, but will (if and when a patch is written here) allow you to choose "skins" on a node level basis. I hope that clarifies things a bit.
Did you mean skin or theme? :-)
Comment #8
k3vin_nl commentedJacine, thanks for your explanation. Your example states a more sensible use of the node-type settings, although I haven't felt the need for this usage so far.
bkosborne, since it looks like this isn't coming to Skinr anytime soon, maybe you could look into building this as an plugin module? I can't program this (I'm more into theming), but I would be more then willing to help with testing, documentation, etc...
Comment #9
shiv.godi commented@K3vin, I am actually working on having skins for specific nodes, this will be my first ever patch, your testing will be very much needed.
I am grappling with some issues and I was hoping to find a solution by this weekend, let’s see, I think I still have some time!
Comment #10
nomonstersinme commentedI'll be happy to test any patches as well!
Comment #11
bkosbornesorry for the confusion. I didn't mean actual theme, but instead just a class, just as it does for blocks
Comment #12
shiv.godi commentedIt will take more time for me to find the fix for the node specific skins, I will get back once I am done with it. Hopefully soon.
Comment #13
shiv.godi commentedJust when I was about to create the patch for the node specific skins, I realized that I was working on [release version 6.x-1.5].
I am going to retain the node specific skin changes to 6.x-1.5, just in case there is a need to port such change to 6.x-1.x-dev, I can do that.
And then when I started working on [6.x-2.x-dev], I found that a lot of code being added to the [6.x-2.x-dev].
I have doubts about the implementation of node specific skins in [6.x-2.x-dev], will someone kindly answer them.
1. I am thinking of providing the option of selecting the skins for the specific node on the node/edit form, is this required or its enough to create rules for each of the page/pages in admin/build/skinr/rules?
2. When skin for the node type is selected, the pages specified in the rules will override the node type skin, is this how it should be implemented?
3. Is it a good idea to have option of selecting skin on the node/edit form? If so, what should be given priority, the skins in the rule for that particular page or the one selected in the node/edit form?
4. In the database table skinr, I am assuming that column sid will hold the skin id/name of the node type as well as the specific node id, is this right?
Thanks
Comment #14
jacineThere's no difference from the 1.x version where this is concerned.
re: 1, 2 & 3:
Rules in 6.x-2.x are just for page rules, which apply body classes. It exists because there is no Drupal UI to configure a page. In Drupal 7, you can also create "rules" for regions in the same place, for the same reason.
Right now, Skinr only allows settings per node type, and these are applied when editing the content type itself. I don't think it would make sense do this anywhere else but in the node/edit form.
Implementing this per node would add a class to node.tpl.php, in addition to whatever Skinr node type class that may have been applied. If someone writes CSS to conflict with a page rule (body class) or any other class, that would be their problem. I really don't think applying Skins by individual nodes is an effective way to use Skinr, but that's besides the point, I guess (sorry, I had to say it).
No, it should match the way the template suggestions work. That would be module = node, sid = nid.
I hope that helps :)
Comment #15
shiv.godi commentedHere is my first patch !
This patch provides options for applying skins to specific nodes.
Just two files have been updated.
Please do give your valuable feedback.
Thanks !
Comment #17
moonray commentedShiv, first off: thanks for trying your hand at a patch.
Now some comments:
• The patch seems to be in reverse order (the additions and subtractions are reversed).
• There are a lot of whitespace changes which are not necessary.
• You might be better off writing a separate "form index handler" for the page_node_form, rather than trying to figure out which you're dealing with in a combined one; there's a lot of code in that node_skinr_form_index_handler() that you won't need for the page_node_form.
• What are the changes you made in skinr.module? None should be needed unless there's a bug...
Comment #18
shiv.godi commentedThank you for your feedback;
"• The patch seems to be in reverse order (the additions and subtractions are reversed)."
I used diff -urp original_directory new_directory > filename.patch command to create the patch. Will check it again.
"• There are a lot of whitespace changes which are not necessary."
Will take care of this.
"• You might be better off writing a separate "form index handler" for the page_node_form, rather than trying to figure out which you're dealing with in a combined one; there's a lot of code in that node_skinr_form_index_handler() that you won't need for the page_node_form."
I will write a separate handler.
"• What are the changes you made in skinr.module? None should be needed unless there's a bug..."
I had a difficulty with this one, let me explain:
Content of zen.info file:
skinr[block_styles][options][1][label] = Flag heading
skinr[block_styles][options][1][class] = flag
skinr[block_styles][options][2][label] = No title/Blue with rounded corners
skinr[block_styles][options][2][class] = no-title blue
skinr[block_styles][options][3][label] = brown/Georgia font
skinr[block_styles][options][3][class] = brown georgia
In file skinr.module:
foreach ($sids as $sid) {
if ($skinr = skinr_get($theme, $module, $sid, $reset)) {
$skins = $skinr->skins + $skins;
}
This code does union of skins with the same key[block_styles] and if page type has class "flag" and node nid has "brown" class. The above code does retain only one of the class as key[block_styles] are the same for both the classes.
Please let me know if I have not explained the problem elaborately.
Thanks
Comment #19
misc commentedAny progress on the patch? I would really love this feature.
EDIT: I have now succeeded to manually ad the patch - but when I edit a node the settings are lost...
Comment #20
moonray commentedSince there's no progress, and it's new functionality, I'm moving it to the D7 branch. This will then have to be back-ported later.
Comment #21
tchopshop commentedI would love this functionality!
Comment #22
ChrisBryant commentedHi Shiv, have you had any chance to further this patch along? I know this issue is moved to the 7.x branch now and that would take some additional work to get it functional in 7.x. Is that something you still plan to do or should this issue be re-assigned? If you won't be able to continue or finish it, please adjust the "assigned" field so that others know and can continue the work if needed. Thanks!
Comment #23
shiv.godi commentedI will work on this issue, I was way and busy for sometime and I will be working on D7. Will upload the patch as soon as I can.
Comment #24
k3vin_nl commentedI didn't succeed in applying your earlier patch manually, but I still am very eager to help test this feature.
Comment #25
coltraneThe patch in #15 does indeed look reversed, as evident by:
--- cygdrive/d/drupal/patches/new/skinr/modules/node.skinr.inc 2010-10-22 12:06:03.090550800 +0530
+++ cygdrive/d/drupal/patches/old/skinr/modules/node.skinr.inc 2010-03-04 19:37:00.000000000 +0530
If patches/new is the new code then you want this to read:
--- old
+++ new
Comment #26
moonray commentedPostponing until we have someone that actually needs this functionality and is willing to work on a patch.
Comment #27
rooby commentedI need this for 7.x-2.x.
I'll probably have to work it out before I get a response but can someone give a brief outline of what would be required as a head start?
Comment #28
rooby commentedTurned out to be super easy.
It works at least. I don't really know if I have done it correctly.
The issue that arises from this is that you now get 2 contextual links for nodes. One for the content type and one for the individual node.
With the current system this is a problem as both links have the same title.
I have added a fix for this but again, I don't know if I have done it the best way. Just a way that works for me at short notice.
It works well except some of the contextual links for things like blocks get a little messy looking.
It is possible it might be better to use this only for node contextual links, and for others just use "Edit !module skins" for other modules.
Another way might be to have a setting to select only one of the two links to be visible via the admin area, however that isn't ideal as some sites might want to have both node type level skins and node level skins.
Also, maybe individual node skins could be something the the admin has to enable before they are available if people don't think it would be a common use case?
Comment #30
rooby commentedDamn, the tests don't like the change of link text.
I can fix that tomorrow or if a maintainer would prefer I can revert the tweak to the link text (although having two contextual links with the same text is definitely not on).
Comment #31
moonray commentedI feel like per node (rather than per node type) targeting needs to be in addition to/separated out from what exists now for the node type.
Additionally, having multiple 'edit skin' links was found to be bad UI (see #1007482: Make use of Context API instead of custom rules. We need to have a similar (but perhaps simplified) UI to when skinr_context_ui module is enabled to handle this in skinr_ui, and the ui needs to work for skinr_context_ui as well.
Comment #32
moonray commentedNew features should be added to D8 version, then back-ported.