I made a few changes, basically these are the items that irritated me and my users:

(1) I want anonymous users to view the wishlist. But then they also had the "get this gift" option which shouldn't appear unless logged-in AND having access wishlist privelege
(2) In the default wishlist view (wishlist/1) it's hard to distinguish what goes with what row because of the center vertical alignment, so I did align top in the css for the table rows. I also changed the color of even and odd rows
(3) removed the last updated field from the default view (wishlist/1) because it made the table too wide

So anyway, I "hacked" my way through my first PHP and CSS edit ... it works, but perhaps there's a better way to do it??

CommentFileSizeAuthor
wishlist.zip11.24 KBmarcham

Comments

scott.mclewin’s picture

scott.mclewin commented

I think these are good changes. As I noted in your other issue, I'd like to see them as patches, one per related set of changes. This way I can apply the patch, test just what it changes and move on to the next. It also allows me to see exactly what you changed in a single summary rather than comparing code locally. You can imagine that after my local development copy has too many other changes in it that it becomes difficult to assess the impact of other alterations.

I didn't realize this was your first set of php and css changes. Thanks for having the interest in wishlist.module to take on something new! It strikes me that you may not be familiar with how patches work either. A patch file is a standard format for expressing changes from a baseline file (i.e. what is in cvs right now on the 4.7 branch) to your own local changes. You create them with the cvs diff command, or just the Unix diff command. Please use the -u option. As in:

cvs diff -u wishlist.module > my_patch_file.patch

or

diff -u wishlist.module > my_patch_file.patch

Thanks. Assuming my new baby arrives at about the right time (very soon now) I plan to start working the wishlist module's issue queue by mid December.

scott.mclewin’s picture

scott.mclewin commented
Status: Active » Fixed

On #1 - I've recently committed a set of new features that likely cover this case. If you were going to roll a patch, grab the latest first and see if it is still relevant. If so create a new issue with a patch on it.

On #2 - Nice. I've added your CSS changes.

On #3 - I've added a feature that makes the presence of all columns configurable. That covers this need.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)