Issue 11999043: Inspector charm details styling.

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 11999043: Inspector charm details styling. (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
10 years, 9 months ago by huwshimi

Modified:
10 years, 9 months ago

Reviewers:
mp+177304, jeff.pihach

Visibility:
Public.

Description

Inspector charm details styling. Styled the inspector charm details and its content to look like the browser charm details. To view the panel: - head to http://0.0.0.0:8888/:flags:/serviceInspector/ - drag ceph into the canvas and deploy with 'confirm' - click on the service icon in the canvas and the inspector loads in the upper right. - click on the 'cs:precise/ceph-13 Charm details' link to open up the charm details view. There are no reference visual designs for the panel so I've just tried to make it look reasonable. The content of the tabs should match that of the charm details in the browser (sidebar mode). https://code.launchpad.net/~huwshimi/juju-gui/panel-styling/+merge/177304 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 3

Created: 10 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+73 lines, -88 lines)			Patch
A	[revision details]	View	1 chunk	+2 lines, -0 lines	0 comments	Download
M	app/subapps/browser/templates/browser_charm.handlebars	View	1 chunk	+0 lines, -1 line	0 comments	Download
M	app/templates/left-breakout-panel.handlebars	View	1 chunk	+1 line, -1 line	0 comments	Download
M	app/templates/service-inspector.handlebars	View	1 chunk	+2 lines, -2 lines	1 comment	Download
M	app/views/viewlet-manager.js	View	2 chunks	+2 lines, -0 lines	1 comment	Download
M	app/views/viewlets/charm-details.js	View	1 chunk	+3 lines, -1 line	1 comment	Download
M	lib/views/browser/charm-full.less	View	2 chunks	+2 lines, -20 lines	0 comments	Download
M	lib/views/browser/main.less	View	2 chunks	+1 line, -18 lines	0 comments	Download
M	lib/views/browser/reset.less	View	1 chunk	+0 lines, -3 lines	0 comments	Download
M	lib/views/browser/section-title.less	View	1 chunk	+1 line, -0 lines	0 comments	Download
M	lib/views/browser/tabview.less	View	2 chunks	+22 lines, -7 lines	0 comments	Download
M	lib/views/juju-inspector.less	View	1 chunk	+15 lines, -12 lines	0 comments	Download
M	lib/views/stylesheet.less	View	3 chunks	+15 lines, -22 lines	0 comments	Download
M	test/test_viewlet_manager.js	View	1 chunk	+7 lines, -1 line	0 comments	Download

Messages

Total messages: 2

Expand All Messages | Collapse All Messages

jeff.pihach

10 years, 9 months ago (2013-07-29 14:29:48 UTC) #2

Looking good! There are a few smaller issues to be fixed up on the code side.

QA was good - when clicking tabs (Readme & Quality) on the first load the
loading spinner shows up in the top left corner on top of the other tabs with
the name of the tab as the content to the body of the tab. 

It would be great if the tab header was sticky and only the body of it scrolled.
Follow-up feature request?

https://codereview.appspot.com/11999043/diff/1/app/templates/service-inspecto...
File app/templates/service-inspector.handlebars (right):

https://codereview.appspot.com/11999043/diff/1/app/templates/service-inspecto...
app/templates/service-inspector.handlebars:1: <div class="yui3-juju-inspector
yui3-skin-sam">
I'd like this moved to the left-breakout or as close as possible to where it is
required so that it doesn't have a negative effect on any other styles in the
app.

https://codereview.appspot.com/11999043/diff/1/app/views/viewlet-manager.js
File app/views/viewlet-manager.js (right):

https://codereview.appspot.com/11999043/diff/1/app/views/viewlet-manager.js#n...
app/views/viewlet-manager.js:439: Y.one('.left-breakout').hide();
This cannot be here because this method handles hiding all slots. The previous
line with remove() should handle removing all of the content.

https://codereview.appspot.com/11999043/diff/1/app/views/viewlets/charm-detai...
File app/views/viewlets/charm-details.js (right):

https://codereview.appspot.com/11999043/diff/1/app/views/viewlets/charm-detai...
app/views/viewlets/charm-details.js:51: var panel = Y.one('.left-breakout');
We should probably be passing a reference to the container to this render method
so we don't have to do this.

Expand All Messages | Collapse All Messages