Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Auto complete for the custom bookmark titles in the address bar #6111

Closed

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Dec 9, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Resolves #6104

Auditors

@bsclifton, @aekeus, @bradleyrichter

Test Plan

  • Save bookmark with a custom title
  • Search in address bar for a custom title
  • You should see a bookmark with a custom title in bookmarks section

Resolves brave#6104

Auditors: @bsclifton, @aekeus @bradleyrichter

Test Plan:
- Save bookmark with a custom title
- Search in address bar for a custom title
- You should see a bookmark with a custom title in bookmarks section
@NejcZdovc
Copy link
Contributor Author

NejcZdovc commented Dec 9, 2016

Current implementation of the suggestion window:

image

As you can see now you have a custom title first testme and then url title in parentheses Bluetooth 5 prispel @Slo-Tech. Do you like this presentation or should we display only custom title?

@bsclifton
Copy link
Member

Reviewer note: this PR can be closed if we remove customTitle (which I believe we should do).

See note in #6108 (comment)

@NejcZdovc
Copy link
Contributor Author

@bsclifton should we then close this issue then? I can start working on removing customTitle, but I am not sure under which issue is this stated. #5738 or #6108 or both 😃

@bsclifton
Copy link
Member

@NejcZdovc that would be great! 😄 I'll go ahead and close

Removing customTitle, per the discussion linked above, should fix:
#6104
#6108

I'm not sure if #5738 is affected; this seems like a different situation. There are likely other issues which have this same root cause. I'll create a new issue for removing customTitle where we can track them all

@bsclifton bsclifton closed this Jan 9, 2017
@NejcZdovc
Copy link
Contributor Author

@bsclifton great, sounds like a plan. You can assign this issue to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants