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

History follow up #3253

Merged
merged 2 commits into from
Aug 19, 2016
Merged

History follow up #3253

merged 2 commits into from
Aug 19, 2016

Conversation

bsclifton
Copy link
Member

Follow up for #3206

cc: @bridiver @bbondy

The only thing not addressed are the IPC calls. I definitely agree we can move:

  • closed frame info
  • active frame location

and more into AppStore (and then we'd just use actions). Those changes are "bigger than a breadbox" so I left them out of this PR.

Here's what's fixed:

  • about:history now supports context menu (shares code w/ bookmarks)
  • added isBookmark to siteUtil and updated code to use it
  • fixed comment on UPDATE_MENU_BOOKMARKED_STATUS
  • Delete history item now works
  • Broke out common styles (between history and bookmarks about pages) to siteDetail.less, updated both to use it
  • Sending menu update (via IPC) only sends what's needed; needs to be refactored to live in appState
  • Updated directory structure for menu, menuUtil, and commonMenu per https://github.com/brave/browser-laptop/blob/master/docs/directoryStructure.md

@bbondy
Copy link
Member

bbondy commented Aug 19, 2016

would you mind rebasing this? It looks like there is a conflict to merge. I still haven't reviewed though btw.

- about:history now supports context menu (shares code w/ bookmarks)
- added isBookmark to siteUtil and updated code to use it
- fixed comment on UPDATE_MENU_BOOKMARKED_STATUS
- Delete history item now works
- Broke out common styles (between history and bookmarks about pages) to siteDetail.less, updated both to use it
- Sending menu update (via IPC) only sends what's needed; needs to be refactored to live in appState
- Updated directory structure for menu, menuUtil, and commonMenu per https://github.com/brave/browser-laptop/blob/master/docs/directoryStructure.md
@bsclifton
Copy link
Member Author

@bbondy done 😄

@bbondy
Copy link
Member

bbondy commented Aug 19, 2016

++

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.

4 participants