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

Changing bookmark folder in the add bookmark modal changes it to bookmark added #5314

Closed
srirambv opened this issue Nov 1, 2016 · 6 comments · Fixed by #9710
Closed

Changing bookmark folder in the add bookmark modal changes it to bookmark added #5314

srirambv opened this issue Nov 1, 2016 · 6 comments · Fixed by #9710

Comments

@srirambv
Copy link
Collaborator

srirambv commented Nov 1, 2016

Test plan

#9710 (comment)


Did you search for similar issues before submitting this one?
Yes

Describe the issue you encountered:
Changing bookmark folder in the add bookmark modal changes it to bookmark added

Expected behavior:
Should just change the folder location and not change modal to bookmark added

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Windows 10 x64

  • Brave Version:
    Verified from 0.12.6 onwards

  • Steps to reproduce:

    1. Open about:bookmarks and create a new bookmark folder
    2. Click on the start+ icon to add a new bookmark
    3. Change the bookmark folder from toolbar to the folder created in step 1/ or vice versa
    4. Modal changes to bookmark added instead of retaining the add new bookmark modal
  • Screenshot if needed:
    bkmrkadded

  • Any related issues:

cc: @bsclifton
Setting milestone to 0.12.9

@srirambv srirambv added this to the 0.12.9dev milestone Nov 1, 2016
@bsclifton
Copy link
Member

Removed milestone- Let's discuss if you feel strongly / want to re-add

@bsclifton bsclifton removed this from the 0.12.9dev milestone Nov 8, 2016
@gyandeeps
Copy link
Contributor

gyandeeps commented Dec 11, 2016

I started working on this and found we use showLocation state value to determine the title of the modal. That is not correct because the title and location show are not directly linked to each other.
Example: you can have a location filled out bu that doesn't mean bookmark has been added.

Plan: Come up with a another state attribute called phase which will have values from new, edit, done so that we can accurately determine the title without interfering with location.

Another issue

Issue: Also fix when the location field is shown because if you look at the visual inside issue, even if you dont have the location set but changing the folder will hide the location option.

Plan: To always show when you are editing bookmarks from bookmark manager. Will not show location if you are interacting with it by clicking the start inside the url bar because at that time you already have the location in context.

@bsclifton
Copy link
Member

@gyandeeps that would definitely work 😄 If you are interested in grabbing this, you could take a look at the action and identify the callers that would have to change. If you need any help or have any questions, let me know 😄

@kumarrishav
Copy link
Contributor

@srirambv It's happening on MacOS Brave 0.15.2 rev af7ef42 too. Also 'location' input box getting disappeared while changing the folder like as @gyandeeps said.
Also, can we add platform label too to the issue? It's easy to filter by platform label and easy to pick the issue as everyone won't have all platform laptop.
Any thoughts on this @bsclifton

@luixxiul
Copy link
Contributor

@kumarrishav basically if an issue is global and not specific to a platform, you don't need to add platform labels.

@luixxiul luixxiul added the bug label Jun 4, 2017
@NejcZdovc NejcZdovc self-assigned this Jul 20, 2017
@NejcZdovc NejcZdovc added this to the 0.19.x (Beta Channel) milestone Jul 20, 2017
@NejcZdovc
Copy link
Contributor

This should be fixed with #9710

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.