Hi Henning!

Thanks for getting in there and getting this page a bit more consistent!

I'm setting this to code-approved - as I'm guessing you've already got
it running through ec2test, but I'd like someone else to second the
ui as I've got some questions about the usage of the registration
slot.

> = Summary =
> 
> Fixes bug 435712 by moving the details about a question to the "registering"
> slot, like it is with bugs and branches.

Right - I hadn't realised that we were putting more than the actual
registration info into that slot - but can see that it's already done
on the bugs page. That is, I hadn't realised we were going to communicate
status and/or repeat the context (bug XXX or question xxx) in that slot.

I personally prefer the branches page, where it is *just* the registration
info:

Created by Michael Nelson on 2009-09-23 and last modified on 2009-09-23

rather than what's on the bugs page:

Bug #201001 reported by Celso Providelo  on 2008-03-11

which I reckon ought to be:

Reported by Celso Providelo  on 2008-03-11

but I haven't been involved in any of the discussions - so perhaps there's
already been a decision there? I just think that yet another reference
to 'Bug #xxxxx' or '...question #xxxx' is unnecessary (and getting
noisy). What are your thoughts? The status could go below with the language
details in a dl (like on other 3-0 pages - the object details).

A note regarding the status - I don't think it's part of this branch,
but I think it should be "Answered question" rather than "Answered Question".
See: https://dev.launchpad.net/UserInterfaceWording#Capitalization

Ah, I just noticed that there's no leaf breadcrumb 'Answered question #xxx'.
But as you said in irc, that's a separate bug.

> The language information was simply
> moved to the main body.
> 
> == Screenshots ==
> 
> Before and after:
> http://people.canonical.com/~henninge/screenshots/answers-
> bug-435712-before.png
> http://people.canonical.com/~henninge/screenshots/answers-bug-435712-after.pn
> 

Thanks for the screenshots!

> == Test command ==
> 
> bin/test -vvt question -t this-is-a-faq
> 
> == Demo/QA ==
> 
> Go to any question and verify that is looks like the "after" screenshot
> avbove.

Great.


> === modified file 'lib/lp/answers/browser/question.py'
> --- lib/lp/answers/browser/question.py	2009-09-22 15:43:52 +0000
> +++ lib/lp/answers/browser/question.py	2009-09-24 14:18:35 +0000
> @@ -755,6 +755,10 @@
>      initial_focus_widget = None
>  
>      @property
> +    def label(self):
> +        return self.context.title
> +
> +    @property
>      def page_title(self):
>          return smartquote('%s question #%d: "%s"') % (
>              self.context.target.displayname,
> 
> === modified file 'lib/lp/answers/stories/question-add.txt'
> --- lib/lp/answers/stories/question-add.txt	2009-09-23 14:40:53 +0000
> +++ lib/lp/answers/stories/question-add.txt	2009-09-24 14:18:35 +0000
> @@ -157,9 +157,9 @@
>      >>> user_browser.title
>      'Questions : \xe2\x80\x9cmozilla-firefox\xe2\x80\x9d package : Ubuntu'
>  
> -    >>> contents = find_main_content(user_browser.contents)
> -    >>> contents.findChild('div', id='question-details')
> +    >>> print find_tag_by_id(user_browser.contents, 'registration')
>      <div...>Open<... Question #..., asked...by...No Privileges Person...

Any reason for not using extract_text here? Might be nicer to read...

> +    >>> contents = find_main_content(user_browser.contents)
>      >>> contents.find('div', 'report')

... and here: print extract_text(contents.find('div', 'report'))

>      <div class="report"><p>I use Ubuntu on AMD64 ...</div>
>  
> 
> === modified file 'lib/lp/answers/stories/question-confirm-url.txt'
> --- lib/lp/answers/stories/question-confirm-url.txt	2009-08-25 21:30:02 +0000
> +++ lib/lp/answers/stories/question-confirm-url.txt	2009-09-24 14:18:35 +0000
> @@ -96,7 +96,7 @@
>  This adds his comment to the question and mark it as 'Solved.'
>  
>      >>> print extract_text(
> -    ...     find_tag_by_id(owner_browser.contents, 'question-details'))
> +    ...     find_tag_by_id(owner_browser.contents, 'registration'))
>      Solved ...

Ah, so it's also communicating the status... - I personally am not
convinced that it belongs there - but maybe you've had discussions
with others about it?

I think it should simply be:

Asked on 2006-01-01 by Guilherme Salgado (and potentially last updated - 
not sure if that's in the db?)

>      >>> print find_tags_by_class(
>      ...     owner_browser.contents, 'boardCommentBody')[-1].renderContents()
> 
> === modified file 'lib/lp/answers/stories/question-edit.txt'
> --- lib/lp/answers/stories/question-edit.txt	2009-08-25 21:30:02 +0000
> +++ lib/lp/answers/stories/question-edit.txt	2009-09-24 14:18:35 +0000
> @@ -41,7 +41,7 @@
>  
>      >>> def print_question_status(browser):
>      ...     print extract_text(
> -    ...         find_tag_by_id(browser.contents, 'question-details'))
> +    ...         find_tag_by_id(browser.contents, 'registration'))
>  
>      >>> user_browser.open('http://launchpad.dev/ubuntu/+question/3')
>      >>> print_question_status(user_browser)
> 
> === modified file 'lib/lp/answers/stories/question-reject-and-change-status.txt'
> --- lib/lp/answers/stories/question-reject-and-change-status.txt	2009-08-25 21:30:02 +0000
> +++ lib/lp/answers/stories/question-reject-and-change-status.txt	2009-09-24 14:18:35 +0000
> @@ -52,7 +52,7 @@
>  
>      >>> def print_question_status(browser):
>      ...     print extract_text(
> -    ...         find_tag_by_id(browser.contents, 'question-details'))
> +    ...         find_tag_by_id(browser.contents, 'registration'))

Hmm... perhaps it's worth moving this into something like
l/c/lp/testing/pages.py? See what you think.

>  
>      >>> print_question_status(admin_browser)
>      Invalid ...
> 
> === modified file 'lib/lp/answers/stories/question-workflow.txt'
> --- lib/lp/answers/stories/question-workflow.txt	2009-09-18 15:24:30 +0000
> +++ lib/lp/answers/stories/question-workflow.txt	2009-09-24 14:18:35 +0000
> @@ -15,7 +15,7 @@
>      # added and the status of the question.
>      >>> def find_request_status(contents):
>      ...     print extract_text(
> -    ...         find_tag_by_id(contents, 'question-details'))
> +    ...         find_tag_by_id(contents, 'registration'))
>  

and again.

>  
>      >>> def  find_last_comment(contents):
> 
> === modified file 'lib/lp/answers/stories/this-is-a-faq.txt'
> --- lib/lp/answers/stories/this-is-a-faq.txt	2009-09-23 14:40:53 +0000
> +++ lib/lp/answers/stories/this-is-a-faq.txt	2009-09-24 14:18:35 +0000
> @@ -104,7 +104,7 @@
>  
>      >>> def print_question_status(browser):
>      ...     print extract_text(
> -    ...         find_tag_by_id(browser.contents, 'question-details'))
> +    ...         find_tag_by_id(browser.contents, 'registration'))
>  

and again.

>      >>> print_question_status(user_browser)
>      Answered ...
> 
> === modified file 'lib/lp/answers/templates/question-index.pt'
> --- lib/lp/answers/templates/question-index.pt	2009-09-03 20:22:39 +0000
> +++ lib/lp/answers/templates/question-index.pt	2009-09-24 14:18:35 +0000
> @@ -18,12 +18,7 @@
>      </style>
>    </head>
>    <body>
> -    <metal:heading fill-slot="heading">
> -      <h1 tal:content="context/title">Bar doesn't work</h1>
> -      <div id="question-details">
> -        <img alt="Open" title="Open" src="/@@/question"
> -          tal:attributes="alt context/status/title;
> -                          title context/status/title;" />
> +    <metal:registering fill-slot="registering">
>          <span class="questionstatusOPEN"
>            tal:attributes="class string:questionstatus${context/status/name}"
>            tal:content="context/status/title">Open</span>
> @@ -48,14 +43,8 @@
>          <a
>            tal:attributes="href context/owner/fmt:url"
>            tal:content="context/owner/displayname">Foo Bar</a>
> -      </div>
> -
> -      <div id="question-lang">
> -        <b>Language:</b>
> -        <span tal:replace="context/language/englishname">English</span><br />
> -      </div>
> -
> -    </metal:heading>
> +    </metal:registering>
> +
>  
>      <metal:portlets fill-slot="side">
>        <div tal:replace="structure context/@@+global-actions" />
> @@ -67,6 +56,11 @@
>  
>      <div metal:fill-slot="main">
>  
> +      <div id="question-lang" style="margin-bottom: 1em">
> +        <b>Language:</b>
> +        <span tal:replace="context/language/englishname">English</span><br />
> +      </div>

Did you try a dl here? It's what we're using elsewhere for this kind of thing.
Personally, as I mentioned above, I'd prefer to see both the language
and the status here using a dl.

> +
>        <tal:description
>          define="global description context/description/fmt:obfuscate-email/fmt:text-to-html" />
>  

Thanks again Henning!

-- 
Michael