Code review comment for lp:~rharding/juju-gui/update-urls
Revision history for this message
| Richard Harding (rharding) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
Comments below. Will have an updated branch in a bit.
https:/
File app/subapps/
https:/
app/subapps/
On 2013/04/15 21:06:34, j.c.sackett wrote:
> Don't we need a route like "/bws/sidebar/id*/* to match things like
the readme
> and interface routes above?
Short answer: yes
Longer: this functionality never worked and needs to be tied into the
rendering state of the charm details so it'll be implemented later.
https:/
File app/subapps/
https:/
app/subapps/
there be no editorial content if
On 2013/04/15 21:06:34, j.c.sackett wrote:
> The sequence of if statements here seem odd; can't we just check the
subpath
> thing, and set our containerID &c based on that, as that will also be
false if
> fullscreen isn't even in the URL?
> It's possible I'm just not really understanding some of this branch,
as I'm
> having trouble figuring out the situation in which we call
renderEditorial but
> don't want to renderEditorial.
Editorial is kind of a 'default' content. It's also going to be rendered
differently if it's for a fullscreen or sidebar view. Because of that we
track it's state here.
Render editorial is called manually from both sidebar() and
fullscreen(). It determines if it should go through with it based on the
current state.
/bws/sidebar/
to fill the sidebar while the charm details loads next to it.
/bws/fullscreen
don't need to render the editorial data so we bypass it.
https:/
File app/subapps/
https:/
app/subapps/
On 2013/04/15 21:06:34, j.c.sackett wrote:
> Does this (and the events change in views/charm.js) have to do with
the routes
> &c change, or just a driveby?
sorry, this was part of some debugging work where I had issues with the
double render causing events to bind twice and thus clicking this would
open and close the comments at once.
In debugging I moved the event to the h3 as a whole vs just the <a> and
missed reverting it, but it works so it didn't come out.
https:/
File app/subapps/
https:/
app/subapps/
node to insert our rendered content into.
On 2013/04/15 21:06:34, j.c.sackett wrote:
> These params don't match; _renderCharmView has "charm" and
isFullScreen, not
> container. It's getting container as an attr.
Sorry, you're correct. This was tweaked and I didn't update the
docstring. Thanks!
https:/
File app/subapps/
https:/
app/subapps/
On 2013/04/15 21:06:34, j.c.sackett wrote:
> Given views/charm.js has this same failure as apiFailure, and we're
using it
> here as a callback, we should probably define it in one place and have
both of
> those places use it.
Yes, my goal is to move this to a view/utils but was trying to keep from
doing too much in the branch. I'll update this.
https:/
File app/templates/
https:/
app/templates/
On 2013/04/15 21:06:34, j.c.sackett wrote:
> Can you update the token's less file to make sure pointer type remains
normal
> now that the entire thing is a link?
This is very temp. Actually you have to carefully click in a blank area
to trigger the link right now. I want to get UX/Huw follow up to fix it
up.
https:/