Closed Bug 1019676 Opened 5 years ago Closed 5 years ago

Project editor: Allow app header to be updated and add gear icon / status indicator

Categories

(DevTools :: WebIDE, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 32

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(2 files, 2 obsolete files)

In the project header, we need to add a gear icon and a "dot" (a little circle) that could turn green/orange/red to show if the project has warnings/errors.
We will need to add a new option for setProjectToAppPath(path, opts) that allows the status dot to change.  We could call this opts.status='error|warning|valid'.  It would default to valid if nothing was passed in.
Attached patch project-header.patch (obsolete) — Splinter Review
Can you please have a look at this?  Here is what the patch does:

1) Adds ability to call setProjectToAppPath multiple times with the same path.  In which case it will just update the header label.  This is needed for the app manager so it can update details about the app.  There is a new test for this.
2) Adds a status indicator circle on the project header and a new option called status ("valid", "invalid", "warning")
3) Changes the layout of the header element to use flexbox which helps with cutting off a long app names while still keeping the stuff anchored to the right (I'll upload a screenshot to explain)
4) Sorry for this since it caused a bunch of line noise - but I realized that we could remove references to widgets.css and that required changing a bunch of the class names from .side-menu-widget to prevent confusion.

Regarding the CSS changes, there is a good chance this tree is going to be replaced with a different tree widget in the future.  So just saying that you probably don't need to spend a ton of time on the CSS if you don't want to.
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Attachment #8434403 - Flags: review?(fayearthur)
Screenshot showing what it looks like with and without the patch applied
Summary: Project editor: Add gear icon and status indicator next to app header → Project editor: Allow app header to be updated and add gear icon / status indicator
Attached patch project-header.patch (obsolete) — Splinter Review
Had a couple of typos in the tests.  New try push: https://tbpl.mozilla.org/?tree=Try&rev=b460dd60f768
Attachment #8434403 - Attachment is obsolete: true
Attachment #8434403 - Flags: review?(fayearthur)
> opts.status='error|warning|valid'

For consistency with the appmanager, could it be:

> opts.validationStatus='unknown|error|warning|valid'

... where unknown would not show any bullet?

Also, not sure if you did this or not, hovering the bullet should show a tooltip: "Validation status: ..."
Comment on attachment 8434430 [details] [diff] [review]
project-header.patch

See Comment 2
Attachment #8434430 - Flags: review?(fayearthur)
(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #5)
> > opts.status='error|warning|valid'
> 
> For consistency with the appmanager, could it be:
> 
> > opts.validationStatus='unknown|error|warning|valid'
> 
> ... where unknown would not show any bullet?
> 

Updated

> Also, not sure if you did this or not, hovering the bullet should show a
> tooltip: "Validation status: ..."

Haven't yet.  Do you already have this string defined somewhere for the app manager?
Comment on attachment 8434430 [details] [diff] [review]
project-header.patch

Review of attachment 8434430 [details] [diff] [review]:
-----------------------------------------------------------------

I'm cool with this. Seems like you could just have an explicit function to change the project status though, like project.updateStatus("valid");

::: browser/devtools/projecteditor/lib/plugins/app-manager/plugin.js
@@ +45,2 @@
>      image.setAttribute("src", url);
> +    optionImage.setAttribute("src", OPTION_URL);

Could also consider just adding the class "devtools-option-toolbarbutton" to the element to give it the cog icon as a background. Either way.
Attachment #8434430 - Flags: review?(fayearthur) → review+
Updated with the new opts.validationStatus name to match app manager.  Fresh try push: https://tbpl.mozilla.org/?tree=Try&rev=90eee1abce6a
Attachment #8434430 - Attachment is obsolete: true
Attachment #8435231 - Flags: review+
(In reply to Heather Arthur [:harth] from comment #8)
> Comment on attachment 8434430 [details] [diff] [review]
> project-header.patch
> 
> Review of attachment 8434430 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm cool with this. Seems like you could just have an explicit function to
> change the project status though, like project.updateStatus("valid");

I'm assuming we will need to respond to other options changes also, like name and app icon.  We could probably have a projecteditor.updateAppOptions(opts) that takes in everything except for `projectOverviewURL`.  I'd be fine with changing when integrating this part with app manager if it makes that simpler.
Keywords: checkin-needed
Blocks: 1021475
https://hg.mozilla.org/integration/fx-team/rev/939350a04d9f
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Blocks: 1021831
https://hg.mozilla.org/mozilla-central/rev/939350a04d9f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 32
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.