Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Update search widget #281

Closed
wants to merge 10 commits into from
Closed

Update search widget #281

wants to merge 10 commits into from

Conversation

mitechie
Copy link
Contributor

@mitechie mitechie commented May 8, 2014

Merge the charm result related views into one.

- Move view.js into charmresults.js
- Move editorial.js into charmresults.js
- Move the search.sj into charmresults.js
- Update the code logic in browser.js to always work through charmresults.js
- Make sure charmresults.js properly handles updating the results only as
required by state info passed in.
- Make this work with and without the `il` feature flag
- Make sure home and activeID work properly in both states (`il` and not)

QA

Use and abuse the sidebar doing search, selecting tokens, hitting home, using direct urls to reload the page, and with and without the 馃帍/il

@jujugui
Copy link
Contributor

jujugui commented May 8, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/896/

@jujugui
Copy link
Contributor

jujugui commented May 8, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/897/

@jujugui
Copy link
Contributor

jujugui commented May 8, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/898/

@jujugui
Copy link
Contributor

jujugui commented May 8, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/899/

@jujugui
Copy link
Contributor

jujugui commented May 8, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/900/

@kadams54
Copy link
Contributor

kadams54 commented May 8, 2014

First round of QA:

  • Progress overlay/indicator is way off: http://cl.ly/image/2F382t3I2E1q
  • Search widget is still present when inspector occupies the sidebar: http://cl.ly/image/473i3R2t0T0l (With the il flag, deploy a charm and then click on it.)
  • This may be related, but when I close the inspector, the charm results do no re-render. (il flag active.)

@jujugui
Copy link
Contributor

jujugui commented May 8, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/903/

@jujugui
Copy link
Contributor

jujugui commented May 8, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/905/

- Move view.js into charmresults.js
- Move editorial.js into charmresults.js
- Move the search.sj into charmresults.js
- Update the code logic in browser.js to always work through charmresults.js
- Make sure charmresults.js properly handles updating the results only as
required by state info passed in.
- Make this work with and without the `il` feature flag
- Make sure home and activeID work properly in both states (`il` and not)
@jujugui
Copy link
Contributor

jujugui commented May 8, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/906/

@hatched
Copy link
Contributor

hatched commented May 8, 2014

I haven't yet done a review but there are a few small issues I found during QA:

  • On initial pageload loading spinner is rendered at 0,0 in the sidebar instead of in the middle but after a search its rendered at 0,50%.
  • performing a search while the charm details is open closes the charm details panel.
  • pressing home while the charm details is open closes the charm details panel.

@mitechie
Copy link
Contributor Author

closing, new branch is up.

I will note:

I have fixed the spinner for now

  • I think closing details on home and search is legit. I think that it's a reset of state in either case and you're doing some other work.

@mitechie mitechie closed this May 16, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants