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

Add multiple selection for about:history #4607

Merged
merged 1 commit into from
Oct 10, 2016
Merged

Conversation

darkdh
Copy link
Member

@darkdh darkdh commented Oct 7, 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).

fix #3949

Auditors: @bsclifton, @bbondy

Test Plan:

For click behavior

  1. First click should select a entry
  2. All the following entries should be selectd with cmd/ctrl + click
  3. Another click without cmd/ctrl should cancel selection

For shift click behavior

  1. Click one entry
  2. Shift + click another entry in the same table
  3. The entry between these two will also be selected

For shift click + cmd/control click

  1. Click one entry as start point
  2. command/control click other entries between start point and end point in next step
  3. Shift + click an entry as end point
  4. The entry between start and end point should be selected

For multi selector

  1. Open about:history
  2. Multi-select entries
  3. Test "Open Links in New Tabs"
  4. Test "Open Links in New Private Tabs"
  5. Test "Open Links in New Session Tabs"
  6. Test "Delete History Entries"

@luixxiul luixxiul added this to the 0.12.5dev milestone Oct 7, 2016
@bsclifton
Copy link
Member

bsclifton commented Oct 7, 2016

Great job, @darkdh 😄

One concern I have is for the selected items. @bradleyrichter, what do you think about the colors?
screen shot 2016-10-07 at 2 09 30 pm

The dark orange is multi-selected, the light orange is hovered. If you hover over a multi-select, it looks like this:
screen shot 2016-10-07 at 2 11 14 pm

It feels good- but the concern I had is readability. We may choose to change the font color (white or lighter color) to make selected items easier to read

}

module.exports.isWindows = () => {
return os.platform() === 'win32'
return os.platform() === 'win32' ||
navigator.platform === 'Win32'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for adding these checks in 😄

@bsclifton
Copy link
Member

Reviewed the code in detail, changes look great 👍

@darkdh
Copy link
Member Author

darkdh commented Oct 8, 2016

Enhanced readability as
screen shot 2016-10-08 at 16 52 22
in dcc8e49

@bbondy
Copy link
Member

bbondy commented Oct 10, 2016

Could you also select Shift + click for range selections?

@darkdh
Copy link
Member Author

darkdh commented Oct 10, 2016

Working on it

@bsclifton
Copy link
Member

++ looks great 😄 Thanks for spending the time to make the tests too

fix #3949

Auditors: @bsclifton, @bbondy

Test Plan:
For click behavior
1. First click should select a entry
2. All the following entries should be selectd with cmd/ctrl + click
3. Another click without cmd/ctrl should cancel selection

1. Click one entry
2. Shift + click another entry in the same table
3. The entry between these two will also be selected

1. Click one entry as start point
2. command/control click other entries between start point and end point in next step
3. Shift + click an entry as end point
4. The entry between start and end point should be selected

Note that range select are not allow across tables

For multi selector
1. Open about:history
2. Multi-select entries
3. Test "Open Links in New Tabs"
4. Test "Open Links in New Private Tabs"
5. Test "Open Links in New Session Tabs"
6. Test "Delete History Entries"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add multi-select deletion and drag and drop in about:history
4 participants