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

Bookmark folder is created left of bookmark item, not right #2771

Closed
luixxiul opened this issue Jul 29, 2016 · 12 comments
Closed

Bookmark folder is created left of bookmark item, not right #2771

luixxiul opened this issue Jul 29, 2016 · 12 comments
Assignees
@NejcZdovc
Labels
bug design A design change, especially one which needs input from the design team. feature/bookmarks QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Milestone
0.20.x (Release C...

Comments

@luixxiul
Copy link
Contributor

luixxiul commented Jul 29, 2016 •
edited
Loading

Test plan

#10136 (comment)

Describe the issue you encountered: Bookmark folder is created left of a bookmarked item. It's the same after moving brave folder in AppData\Roaming.

Expected behavior: It should be created right of the item
@luixxiul luixxiul added this to the 0.11.2dev milestone Jul 29, 2016
@bbondy
Copy link
Member

bbondy commented Jul 29, 2016

Valid bug, but it won't block 0.11.2 release. It seems to always put it at the right spot except when it is the last item it puts it second last.

@bbondy bbondy removed this from the 0.11.2dev milestone Jul 29, 2016
@anseljh
Copy link
Contributor

anseljh commented Aug 3, 2016

I noticed this too. New bookmark folders seem to always be put in the next-to-last position.

@luixxiul luixxiul added the bug label Aug 4, 2016
@alexwykoff alexwykoff added the design A design change, especially one which needs input from the design team. label Aug 23, 2016
@alexwykoff
Copy link
Contributor

@bradleyrichter this is worth a moment of design thought. Perhaps all folders should go to the far left of the bookmarks tray?

@bradleyrichter
Copy link
Contributor

Appearing on the left is the Safari way. Appearing on the right is the Chrome/Firefox way.

On a crowded bar, the Left approach insures that it can be seen when added. But the cost is pushing things into the chevron menu.

Following the most popular browsers usually creates less user friction even when there is a slightly better way. So on this one, let's fix this bug, and follow chrome/FX.

@anseljh
Copy link
Contributor

anseljh commented Aug 23, 2016

Just to make sure you guys are understanding the bug:

  • I start with N items on my bookmarks bar.
  • I right-click the bookmarks bar and select "Add Folder..."
  • I type in a name for my new folder ("Stuff") and click "Save".
  • My new "Stuff" folder is in the Nth position. The item that was previously Nth is now N+1th.

image

image

What makes sense to me is to always place the new item last (N+1th), and then let the user move it somewhere else if they want to.

@bradleyrichter
Copy link
Contributor

@anseljh yes, understood.

@alexwykoff was suggesting/questioning changing the behavior which would also fix this bug. But I am suggesting that we fix it as you explained. New items should be last. Not second-last.

@luixxiul luixxiul added this to the 1.1.0 milestone Nov 10, 2016
@alexwykoff
Copy link
Contributor

Still an issue in 0.13.0

add_bookmark_folder_left

@alexwykoff alexwykoff modified the milestones: 1.0.0, 1.1.0 Jan 3, 2017
@bradleyrichter
Copy link
Contributor

Nice use of my novel in-screen-shot messaging approach!

@NejcZdovc NejcZdovc modified the milestones: 0.20.x (Developer Channel), 1.0.0 Jul 20, 2017
@NejcZdovc NejcZdovc self-assigned this Jul 20, 2017
@NejcZdovc NejcZdovc mentioned this issue Jul 20, 2017
Closed
8 tasks
@NejcZdovc
Copy link
Contributor

NejcZdovc commented Jul 20, 2017 •
edited
Loading

This should be fixed in #2771 #10136

@luixxiul
Copy link
Contributor Author

#2771 is fixed with #2771??

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Jul 20, 2017 •
edited
Loading

Inception 😄 Sorry I wanted to say #10136

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 22, 2017
@NejcZdovc
Splits sites object in the app state
1678bf9 
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 22, 2017
@NejcZdovc
Splits sites object in the app state
0802623 
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 25, 2017
@NejcZdovc
Splits sites object in the app state
6696e4b 
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 25, 2017
@NejcZdovc
Splits sites object in the app state
bda4f31 
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 25, 2017
@NejcZdovc
Splits sites object in the app state
94a95b0 
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jul 26, 2017
@NejcZdovc
Splits sites object in the app state
da5b46b 
Resolves brave#1856
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5699
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
@NejcZdovc NejcZdovc mentioned this issue Jul 26, 2017
Merged
8 tasks
NejcZdovc added a commit that referenced this issue Aug 3, 2017
@NejcZdovc
Splits sites object in the app state
f618f0e 
Resolves #1856
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5699
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
@NejcZdovc NejcZdovc modified the milestones: 0.21.x (Nightly Channel), 0.20.x (Developer Channel) Aug 4, 2017
NejcZdovc added a commit that referenced this issue Aug 4, 2017
@NejcZdovc
Splits sites object in the app state
c2fd650 
Resolves #1646
Resolves #1856
Resolves #2655
Resolves #2771
Resolves #3646
Resolves #3694
Resolves #4224
Resolves #4260
Resolves #4833
Resolves #4868
Resolves #4929
Resolves #5072
Resolves #5699
Resolves #5382
Resolves #6104
Resolves #6108
Resolves #6585
Resolves #8022
Resolves #9301
Resolves #9326
Resolves #9978
Resolves #10026

Auditors:

Test Plan:
dfperry5 pushed a commit to dfperry5/browser-laptop that referenced this issue Aug 18, 2017
@NejcZdovc @dfperry5
Splits sites object in the app state
33861b7 
Resolves brave#1646
Resolves brave#1856
Resolves brave#2655
Resolves brave#2771
Resolves brave#3646
Resolves brave#3694
Resolves brave#4224
Resolves brave#4260
Resolves brave#4833
Resolves brave#4868
Resolves brave#4929
Resolves brave#5072
Resolves brave#5699
Resolves brave#5382
Resolves brave#6104
Resolves brave#6108
Resolves brave#6585
Resolves brave#8022
Resolves brave#9301
Resolves brave#9326
Resolves brave#9978
Resolves brave#10026

Auditors:

Test Plan:
@bbondy bbondy modified the milestones: 0.21.x (Developer Channel), 0.20.x (Beta Channel) Oct 25, 2017
This was referenced Dec 20, 2017
Closed
Closed
@srirambv
Copy link
Collaborator

srirambv commented Dec 22, 2017 •
edited
Loading

QA blocked on #11728. Removing Linux check needs to be tested on all platforms once #11728 is fixed

This was referenced Dec 27, 2017
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Jan 9, 2018
Closed
Closed
@LaurenWags LaurenWags mentioned this issue Jan 22, 2018
Closed
19 tasks
@kjozwiak kjozwiak mentioned this issue Jan 31, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@NejcZdovc NejcZdovc

Labels
bug design A design change, especially one which needs input from the design team. feature/bookmarks QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/include
Projects
None yet
Milestone
0.20.x (Release Channel)
Development

No branches or pull requests
9 participants
@bbondy @alexwykoff @anseljh @luixxiul @bsclifton @NejcZdovc @bradleyrichter @srirambv @LaurenWags