Merge lp://qastaging/~midori/midori/headerBarSupport into lp://qastaging/midori

Proposed by Cris Dywan
Status: Merged
Approved by: Paweł Forysiuk
Approved revision: 6968
Merged at revision: 6961
Proposed branch: lp://qastaging/~midori/midori/headerBarSupport
Merge into: lp://qastaging/midori
Diff against target: 1091 lines (+370/-376)
12 files modified
extensions/adblock/extension.vala (+2/-5)
extensions/adblock/widgets.vala (+38/-72)
extensions/statusbar-features.c (+4/-7)
midori/midori-browser.c (+58/-289)
midori/midori-browser.h (+1/-1)
midori/midori-contextaction.vala (+12/-0)
midori/midori-frontend.c (+3/-1)
midori/midori-panedaction.vala (+4/-0)
midori/midori-preferences.c (+7/-0)
midori/midori-window.vala (+239/-0)
midori/midori.vapi (+1/-1)
po/POTFILES.in (+1/-0)
To merge this branch: bzr merge lp://qastaging/~midori/midori/headerBarSupport
Reviewer Review Type Date Requested Status
Paweł Forysiuk Approve
Danielle Foré (community) ux, testing Approve
Review via email: mp+260017@code.qastaging.launchpad.net

Commit message

Implement Midori.Window class with toolbar/ headerbar

To post a comment you must log in.
Revision history for this message
Danielle Foré (danrabbit) wrote :

Packing the urlbar into the title are looks weird and means autocompletion results just aren't legible. I think it would make more sense to just pack it left and expand like you'd do in a regular toolbar.

Since the "Toolbar Editor" extension exposes itself through right clicking on the toolbar, there's now no way to use this extension. It should probably get a little settings icon like other extensions with settings have

review: Needs Fixing
6958. By Cris Dywan

Fix position of extra actions and paned children

6959. By Cris Dywan

Implement search entry width correctly

6960. By Cris Dywan

Use max width instead of width for URL entry

Revision history for this message
Cris Dywan (kalikiana) wrote :

I'm not sure if I understand the comments fully - I changed the property used to size the URL entry so it expands fully. Is that what you meant by packing?

Right-click on buttons works perfectly for me, like with the classic toolbar, you get the menu to toggle toolbars and the toolbar editor is the last item in it.

Revision history for this message
Danielle Foré (danrabbit) wrote :

URL bar looks right here now :)

Can you possibly add a 1px top and bottom margin to the url entry? It looks like without that box-shadow is clipped.

Ah, if you right click the items themselves you'll get the menu. But if you click the headerbar itself, you only get the window menu. I guess I can file this separately. I personally think it's a little counter-intuitive.

I'm not sure how much you care about the separate web search these days. I added it just for kicks and it's really tiny. Also, there's no pane handle to resize like there used to be.

6961. By Cris Dywan

Add and bottom margin to headerbar title

Revision history for this message
Danielle Foré (danrabbit) wrote :

Hey Christian, the last revision didn't seem to fix the issue. The space needs to be reserved for the entry itself. So instead of headerbar.custom_title, you would need to use (widget as Gtk.Entry) directly.

6962. By Cris Dywan

No toolbar style without a toolbar

6963. By Cris Dywan

Use original order of actions in HeaderBar

6964. By Cris Dywan

Pixel margins for the entry

Revision history for this message
Cris Dywan (kalikiana) wrote :

Right-click to edit has always been on the buttons themselves. Also it's not officially possible to extend the headerbar context menu - so I'd say that should be discussed and dealt with separately.

Revision history for this message
Cris Dywan (kalikiana) wrote :

Regarding resizing of web search: I'm not sure if it's worth porting the existing design as-is because it can only work if both entries sit in the headerbar which seems conceptually wrong. If interest is there it can certainly be looked into later.

Revision history for this message
Danielle Foré (danrabbit) wrote :

Yeah I'm of the opinion still that the separate web search should just be removed since that functionality is already provided in the urlbar. Just wanted to test possible cases :)

I agree the right click menu bit can/should be a different issue.

Revision history for this message
Danielle Foré (danrabbit) wrote :

Crashes when enabling "statusbar features" extension:

ERROR:/home/daniel/Projects/headerBarSupport/extensions/statusbar-features.c:59:statusbar_features_toolbar_notify_toolbar_style_cb: code should not be reached

Revision history for this message
Danielle Foré (danrabbit) wrote :

Thoughts on moving the extensions area of the toolbar to the right? This would seem to follow with Chrome and Firefox

6965. By Cris Dywan

Let statusbar features handle toolbar not being toolbar

6966. By Cris Dywan

Always put extra actions on the headerbar's tail

Revision history for this message
Danielle Foré (danrabbit) wrote :

Everything working as expected here :D

review: Approve (ux, testing)
Revision history for this message
Cody Garver (codygarver) wrote :

I made some comments throughout the diff about code style inconsistencies

Be sure to update the translation template after you commit new string(s) to trunk

6967. By Cris Dywan

Fix spacing in lambdas

6968. By Cris Dywan

Only connect buttons to toolbar style

Revision history for this message
Paweł Forysiuk (tuxator) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: