Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

address bar context menu #153

Merged
merged 6 commits into from Mar 29, 2019

Conversation

Projects
6 participants
@balcy
Copy link
Collaborator

commented Feb 21, 2019

  • new address bar dialog (copy url, paste & go, select all)
  • add "copy url" to browser menu, change some positions
  • fix empty favicons in the HistoryModel

(see suggestions in #64)

@UniversalSuperBox

This comment has been minimized.

Copy link
Member

commented Feb 22, 2019

I'll need to defer to @cibersheep on this one. I feel like the overflow menu is starting to, erm, overflow.

Menu that appears when you long-press on the unselected address bar:

image

Overflow menu:

image

@cibersheep

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2019

We need a plan here. As Dalton pointed, the main menu has become too busy, we have 3 different layout for main menu, copy & paste in the address bar, copy paste in the body and the address bar pop up you present.

Talking about this pop up: I don't think is needed as if you tap on the address bar and log tap on the url you can already copy it, (x) delete it and paste, etc. I think it adds more busyness

@balcy

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 23, 2019

ok so the initial feedback for this was #64 (comment)

The reason for the new dialog (for me) was:

  • there was no long press (right click) action for the address bar before
  • pasting an url quickly now takes a tap, long press, tap on "paste", tap on enter
    the new menu would only take a long press, and tap on "paste & go"
  • the paste & go is available for firefox / other browsers too

does s.o. know if it is possible to make the textbox-context-menu appear on a single long press / right click without selecting the text first ?

Just doing nothing for a long press or doing the same as a normal tap does (select all text) seems wrong, too.

@cibersheep

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2019

I misunderstood the use, I thought you added an action to the main menu.

  • Check the code of the address bar as it selects the whole text on single tap.
  • I wouldn't use a popup for the menu, instead I would add paste&go to the copy/paste menu
    imatge when long tapping on the address bar
  • I would not add a Copy url on the main menu, just use the above copy/paste menu

@balcy balcy changed the title address bar dialog, favicon fix address bar dialog Feb 27, 2019

@balcy balcy changed the title address bar dialog address bar context menu Feb 27, 2019

@balcy

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 27, 2019

new version: (long press on unselected address bar)
image

note: the other context menu for the text box (when already selected) is part of the UI toolkit and cannot be easily extended IMHO

@balcy balcy force-pushed the balcy:xenial_-_addressbar branch from faf520b to b66ada0 Feb 27, 2019

balcy added some commits Feb 21, 2019

- fix empty favicons in the HistoryModel
- new address bar dialog (copy url, paste & go, select all)
- add "copy url" to browser menu, change some positions
revert changes to main menu, no "copy current url" there
instead of an address bar dialog, create an address bar context menu

@UniversalSuperBox UniversalSuperBox self-requested a review Mar 27, 2019

@UniversalSuperBox
Copy link
Member

left a comment

A bit unfortunate that we can't add things to the general context menu, but that's handled by Maliit and we need to think about how we handle it pretty soon anyway. Something is better than nothing!

I'm sorry about the delay to review this again. Feel free to ping me if something goes this long again

@UniversalSuperBox UniversalSuperBox merged commit daf18c0 into ubports:xenial Mar 29, 2019

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details

@UniversalSuperBox UniversalSuperBox added this to In progress in OTA-9 via automation Mar 29, 2019

@cibersheep

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2019

Recently I've looked into the context menu code. Is in here
Adding an Action for every element you want. the target is the element you press and hold on to but it will be on every context menu ;)

@balcy balcy deleted the balcy:xenial_-_addressbar branch Mar 30, 2019

@UniversalSuperBox UniversalSuperBox moved this from In progress to QA in OTA-9 Apr 18, 2019

@Danfro

This comment has been minimized.

Copy link

commented May 1, 2019

Testing on E5, RC channel 2019-W18.

Context menu mostly works fine. Two things to mention:

  1. "paste and go" seems to work only on the start-page with https://duckduckgo.com. I could not get this option at another point/page.
    I did select all on a bookmark before testing with press and hold to raise the context menu. I never got "paste & go" except on the start-page.
    Especially after clearing the address via "X" icon you would expect "paste&go" to be available.

  2. a) "Select all" is ONLY available if there is just the single cursor (no other selection) AND you press & hold onto that cursor. It would be great if press/hold somewhere else could do that as well and if "select all" would be available also with another selection. Basically have it as standard button available in the context menu?

  3. b) After pressing "select all" the context menu for cut/copy/paste appears. But this is located somewhat to the right hand side of the screen. With me being right handed, that is where my hand was still hovering. It took me three attempts to realise that the context menu appeared because it was hidden underneath my hand. Could the context menu always pop up in the centre of the screen? I am not suggesting on the left hand side because that would just cause the same issue for every on left handed.

@UniversalSuperBox

This comment has been minimized.

Copy link
Member

commented May 2, 2019

@Danfro,

The "Paste and Go" only appears if the address bar is deselected and you press and hold on it in the deselected state. It will not appear if you have already selected text or have any cursor in the field.

Other issues with selection behavior are system-wide and could be filed on https://github.com/ubports/ubuntu-ui-toolkit.

@Danfro

This comment has been minimized.

Copy link

commented May 2, 2019

@UniversalSuperBox
Even if the cursor is within the address bar (without actual selection) there is no "paste and go".
That's what I think is not intuitive. Why only if address bar is entirely deselected? Why does it need this restriction? I do not see the need for this.

@UniversalSuperBox

This comment has been minimized.

Copy link
Member

commented May 2, 2019

See the comment above, #153 (comment)

@Danfro

This comment has been minimized.

Copy link

commented May 2, 2019

OK. Sorry. Did miss that note. ;-)

@jezek

This comment has been minimized.

Copy link

commented May 5, 2019

FP2 (2019-W18)

context menu works

@ziggutas

This comment has been minimized.

Copy link

commented May 6, 2019

BQ E4.5 rc (2019-W18)
Long press on deselected address bar offers the 'paste and go' option

@ziggutas

This comment has been minimized.

Copy link

commented May 7, 2019

Nexus 7 (flo) rc (2019-W18)
context menu works

@UniversalSuperBox UniversalSuperBox moved this from QA to Done in OTA-9 May 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.