Code review comment for lp:~brian-murray/launchpad/limited-subscriptions-page

Revision history for this message
Curtis Hovey (sinzui) wrote :

I think Salgado is review of the UI is very good. This is a much needed feature, but it introduces a lot of UI confusion. This feature needs a feature flag to ensure lpnet users do not see this until it is ready.

This UI will get complicated in the next year because we do need to move blueprint and answers subscriptions into this page. I think AJAX is going to be a requirement by then.

I do not think this can land until some of the obvious UI confusion is addressed.

You can use this mixin so that the view returns the user to the correct page:
    from canonical.launchpad.webapp.launchpadform import ReturnToReferrerMixin
But I think switching to AJAX will be the only way to address the clarity of the operation that the user needs to accomplish--users can be in 50 teams and that will distract the user from his task.

Do not show me remove icon when the page is an edit operations (switching to AJAX will fix this).

Do not show me edit operations for other users and teams I am not a member of.

The story is not a story. It is not written from the perspective of a user who visits Lp to accomplish a task. I expect to see the user arrive at his profile page, choose the link to +subscriptions, then use a remove link to remove the subscription, then he sees the bug is not listed. I suppose we want a similar story for removing a subscription for a user's team. The conditions for not showing the bug listing belong in a test for the view

I have read the story and I have no idea how I will discover the link to this page. I assume we are not publishing the link in pages to prevent users from seeing the link until we are ready. If this is the case you need an XXX in the story explaining why the user does not start on the page that directs the user to look at his subscriptions. The other course is to use a feature flag that hides the link and the page until we are ready.

review: Needs Fixing (ui)

« Back to merge proposal