Closed (fixed)
Project:
Node Relativity
Version:
master
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
2 Apr 2007 at 23:24 UTC
Updated:
24 Apr 2007 at 16:30 UTC
Jump to comment: Most recent file
I've made this patch by hand - so please check it's format is ok before applying it. I would appreciate feedback as I can't apply this patch myself so I hope it doesn't have bugs in it ;)
All it does is add a class tag to the attributes array of each a link that relativity spits out. To reference these from a css file:
a.relativity_create_new
a.relativity_attach
a.relativity_remove
a.relativity_parent_node
a.relativity_child_node
a.relativity_ancestor_node
a.relativity_link
a.relativity_advanced_settings
a.relativity_display_settings
It also removes this typo in the theme_relativity_link function. This patch removes the class from the div altogether (but leaves the div in place), and applyies the class to the link itself instead. Before darius complains ;) I would point out that the current tag is dysfunctional, so no-one is likely to miss it!
@ 1441
- $output = '<div span="relativity_link">';
Hope it's OK
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | relativity.css_3.patch | 5.06 KB | JohnG-1 |
| #3 | relativity.css_2.patch | 2.9 KB | JohnG-1 |
| relativity.css_.patch | 2.95 KB | JohnG-1 |
Comments
Comment #1
JohnG-1 commentedoops! there's some mistakes in that patch ... hang on I'm working on a second version ...
Comment #2
darius commentedJohn, why by hand? What's your setup - Windows, Linux? There's got to be an easier way :)
I'm not sure we need special attributes for admin section display (a.relativity_advanced_settings, a.relativity_display_settings)? I would rather only change the user view items.
Also, it's always best to file separate small issues for even the smallest bugs. It's easier to review and will be applied faster.
Comment #3
JohnG-1 commentedOK here's version 2. I re-factored the line numbers to fit relativity.module,v 1.40 2007/04/02 18:59:52. Again be aware that it is a handwritten patch, so apply with caution.
1. Related Node links: once I started delving into DIV tags I realised that there were some inconsistencies, which I've tried to iron out. With this patch, each link to a related node is wrapped in a DIV class .relativity_child, .relativity_parent or .relativity_ancestor, depending which list it appears in. Within that DIV, each link is given the class a.relativity_view_node because that is the function of the link.
2. The 'Create' and 'Attach' Operator links are given the A class a.relativity_create and a.relativity_attach. I've not wrapped them in DIVs because this would interfere with their inline display. I have also corrected (and simplified) the insertion of the pipe | separator so it appears only when both Create and Attach links are present. I have re-introduced capitalisation to Create new ... and Attach existing ... labels, as per Drupal convention.
3. The 'Remove' Operator links I've rewired quite substantially:
- Each item in the list of Removables is enclosed within a DIV class .relativity_removable to be consistent with all other Relativity node lists.
- The link now looks like this: Remove page : Title of child page node. Only the 'Remove page' part is the operator link - the title is not linked at all. I did this because it was too easy to hit the old Remove link when you wanted to view the child node ... I was doing it all the time! ('page' of course is the child $type variable.)
4 Some things I toyed with but didn't include in the patch :
a) Building the removable node titles as relativity_view_node links, so the Link Operations Block could be used in place of the Child Nodes Block. This worked pretty well - for users without the operator link permissions it looks exactly like the Child Node list. The problem is that there is a filter on the $removables array that prevents child nodes involved in common child relationships from being displayed. Can't really fix this without getting bogged down in the permission-check system.
b) I looked into re-ordering the Link Operations lists so all three are grouped by child $type ... this should be a themable option but was basically more trouble than it was worth and really requires substantial restructuring of the whole module theme-function layout!- beyond the scope of this patch ;)
c) As far as I can tell, the three 'Helper Theme' functions at the end of the module are completely redundant - they are not called from anywhere in the module. This patch doesn't touch them, but I thought I'd point out why I haven't corrected the typos in those functions.
I think that's everything ;)
Comment #4
JohnG-1 commented@ #2 darius ... lol you posted while I was composing ...
Making and applying patches: my test sites are on a linux machine, and I'm typing in windows. There's no patch utility available on my hostserver. I had a go at tortoise CVS one afternoon but lost patience. I spent the evening searching for something with a simple drag & drop interface that I could run on windows and just upload for testing, but I couldn't find anything! If you have suggestions I am all ears ;)
Small patches: yeah I see what you mean, but when I'm knitting them myself, it's a lot easier to put more into one patch. This patch attempts to address the css tag issue. First I tried the shotgun approach (agreed, links in settings help don't need css tags ;) Then I realised that some of the DIV tags had class="" typos, so I looked into that and found that DIV wrappers were not being output consistently (helps for write clean css files if the block structure is consistent), to I revamped that too ... In patch2, the div tags take over the distinction between parent, ancestor and child contexts, which makes more sense than tagging the A link for this.
I may have got carried away with reformatting the Remove links ... but I do think it's better UI, and more consistent with the other relativity operator links.
Comment #5
darius commentedJohn, we need to figure out this patch/diff issue, otherwise it's unnecessary extra work for both of us :) If you have access to the shell, try:
diff -u oldfile newfile > mypatch.patch
If not, there is definitely some Windows software that will do the same (or get a hosting company that allows shell access).
BTW, the remove link changes make sense.
P.S. If you can't generate the diff output, just attach the whole file with new changes also. Also, Check http://drupal.org/patch/create
Comment #6
JohnG-1 commentedyay! I got it at last! thanks for cajoling me into it darius ;) Also I must apologise for all 8 deal-breaker typos in the last pseudo patch - no wonder you had to say something!
Please find attached a real genuine bonafide patch that makes the world a better place altogether (well, slightly) ... but mostly it will make darius' life a little easier ;)
I'm not going to list the css tags again - if you care: use Firefox's Web Developer Toolbar plug-in...
ah go on then:
one DIV .relativity_child around each child list item.
one DIV .relativity_parent around each parent list item.
one DIV .relativity_ancestor around each ancestor list item.
one DIV .relativity_add_child around each pair of Create and Attach links.
one DIV .relativity_removable around each item on the removables list.
one a.relativity_view_$type for each ordinary link
one a.relativity_create_$type for each Create new link
one a.relativity_attach_$type for each Attach link
one a.relativity_remove_$type for each Remove link
($type is the content type)
The pipe seperator between Create | Attach links is fixed and
The re-factored Removeables list is in (I'm not 100% about the italics ...)
I think that's quite enough for one patch ;)
Comment #7
darius commentedCommitted to CVs HEAD and 5.x-2.x-dev branch. Thanks, John.
Comment #8
darius commentedCommitted to CVS HEAD and 5.x-2.x-dev branch. Thanks, John.
Comment #9
(not verified) commented