Code review comment for lp://staging/~artmello/webbrowser-app/webbrowser-app-bookmark_folders

Revision history for this message
Olivier Tilloy (osomon) wrote :

> > After resolving the trivial build issue pointed out above, I’m doing some
> > quick functional testing, and creating a new bookmark folder doesn’t seem to
> > work: I’m bookmarking a page that wasn’t previously bookmarked, the popover
> > appears, I click on "New Folder", enter a new name (btw pressing the Enter
> key
> > should validate), click "Choose Folder", but the dropdown list still has
> only
> > "All Bookmarks".
>
> The BookmarkOptions component had 2 states: choosing an existing folder and
> creating a new folder. We would only create a new folder if the user dismiss
> the component in the "creating a new folder" state. So when you clicked
> "Choose Folder" you cancelled the creation of the new folder. But, as we can
> see, that is not user friendly. I changed to show a dialog when you click "New
> Folder" and when it is confirmed we will create the folder and it will show up
> in the dropdown. Please, let me know what you think about it.

This looks better, although I would prefer not to have an extra button. Maybe a special entry in the dropdown to create a new folder? In any case design will have to comment on this, so let’s leave it as is for now, it works.

« Back to merge proposal