226696 - [patch] Button/Icon to delete bookmark/folder

Status: RESOLVED FIXED

(Camino Graveyard :: Bookmarks, enhancement)

Description

[Noemi]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.6b) Gecko/20031121 Camino/0.7+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.6b) Gecko/20031121 Camino/0.7+

It would be handy-dandy to have a "delete" button next to the "add bookmark",
"add folder", and "add separator" buttons in the bookmarks window.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Jasper]

I agree this button should be added! Though I think it should be grouped withe
the "reveal" and "info" buttons.

Comment 2

[Noemi]

If someone would create the graphic, I'll tke care of adding it to the .nib file
and hooking it up.

Comment 3

[Jasper]

Noemi the graphic is already in the Camino resource folder > delete_on

Comment 4

[Noemi]

Oops - thanks, Jasper.

I've created an updated .nib file, but I'm having trouble uploading it as an
attachment for some reason - the "Create New Attachment" form is doing nothing.

Comment 5

[Noemi]

Attachment: Updated .nib file

[zipped].

This is the .nib file with the button added and the deleteBookmarks: method set
as its outlet.

Comment 6

[Jasper]

Your nib works but you will need to add code to makes sure that the buttons is
correctly dimmed depending on what is selected.

Comment 7

[Josh Aas]

Attachment: updated nib

Comment 8

[Josh Aas]

Attachment: enable/disable delete button appropriately and a bit of cleanup

Comment 9

[Mike Pinkerton (not reading bugmail)]

after seeing this applied, i'm not sure we need this enough to add yet more
chrome to the manager. it's starting to get crowded.

what i would prefer to see is the "Edit:Delete" menu item work, coupled with
backspace (which already works).

if we do go with this patch, we should enable the button when history items are
selected and disable it for the top-ten (which should be read-only, IMHO).

jasper, convince me why we need this extra chrome.

Comment 10

[Mike Pinkerton (not reading bugmail)]

...er, jasper didn't file this. noemi, can you convince me why we need this?

Comment 11

[Jasper]

Delete is generally used as often if not more often than the info or reveal
button so appart from estehtical issues I don't see why we sholdn't add it. Why
don't we moev the reveal button to the results contextual menu? That would make
more sence thgan keeping out the delete button for esthetical reasons.

The button we now use in the BM are huge, there are better alternatives if we
think it looks to crowded. One of them is using custom button graphics, shown
here http://www.jasperhauser.nl/camino/bla/bmtoolbar.jpg

Comment 12

[Jasper]

Mike as you have seen in the upcomming icon theme the delete button WIL NOT
cause any clutter. 

If the patch is good all that needs to be done is make a new nib cause Mike
tweeked in this week. I will have the icon set ready really soon.

Comment 13

[Jasper]

Attachment: new nib file

- updated the nib to reflect the latest nib changes mike made
- included the new bm icons that go with the new theme that will land soon

Icon changes:
bookmark_on -> bm_book.tif
delete_on -> bm_del
info_on -> bm_info
searchicon -> bm_reveal
separator_on -> bm_sep

Icons that can be removed:
- disclosureArrowDown.tiff (no longer in use in the dl manager)
- disclosureArrowRight.tiff (no longer in use in the dl manager)
- edit_on

We need to email Reinhold Penner again about the icon changes.

Comment 14

[Jasper]

Oeps and:
folder_on -> bm_folder

Comment 15

[Jasper]

Attachment: new nib, removed the delete icon :)

Since we decided not to use an icon for this feature, but instead to fix the
edit menu item states for bookmarks (Bug 155484), the delete icon was removed.

Josh needs to look at his patch and isolate the button state clean ups he did
so we can check that in separatly. Then we can close this bug.

Comment 16

[Mike Pinkerton (not reading bugmail)]

landed new nib, new icons, and removed unused icons. 

Comment 17

[Ludovic Hirlimann [:Usul]]

Does the paatch attached still needs review ?

Comment 18

[Stuart Morgan]

Comment on attachment 140320 [details] [diff] [review]
enable/disable delete button appropriately and a bit of cleanup

Removing review flags on obsolete patch

Comment 19

[Stuart Morgan]

Attachment: Just the cleanup

He wasn't kidding when he said *minor* cleanup; hardly worth keeping this bug
open for. FWIW, here's just the cleanup part of the code.

Comment 20

[Stuart Morgan]

Pushing off the 0.8 list, since what's left is of trivial importance.

Comment 21

[Josh Aas]

Could either Jasper, Mike, or Stuart please explain what is going on with this
bug? Why is there a nib that removes a delete icon that was never added? Do we
just need to land a little cleanup patch?

It seems to me we should have a delete button and the edit menu should work.
Either way this bug should be easy to knock off the 0.9 list.

Comment 22

[Josh Aas]

Comment on attachment 144917 [details] [diff] [review]
Just the cleanup

The cleanup is not necessary. Its already fixed or it doesn't make much sense.

Comment 23

[Josh Aas]

Leaving this open since I think we should have a delete button.

Comment 24

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Is this bug still valid after smfr's rework of the icons/options available in Jan?

There are now buttons/popups (3) for adding items, "modifying" items (delete,
open in tabs, get info), and sorting items.  Is a separate delete button/icon
still warranted?

Comment 25

[Jasper]

I think we can savely close this bug now, Smokey Ardisson is right that the new
buttons we now have already contain any option we have a or have room for
options we might want to add.

We don't need clutter by adding an extra button. So I'm closing this bug.
Marking fixed as the option is available from the "action" popup menu.