Skip to content

Commit

Permalink
Notes: add profile pref storing the sort order for notes ui in side p…
Browse files Browse the repository at this point in the history
…anel.

Bug: 1394044
Change-Id: Ia7dda2c5deb193a894b4e579640c32e226c25335
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4246680
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Yuheng Huang <yuhengh@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1108316}
  • Loading branch information
Caroline Rising authored and Chromium LUCI CQ committed Feb 22, 2023
1 parent 5f067d0 commit 065085a
Show file tree
Hide file tree
Showing 16 changed files with 156 additions and 3 deletions.
1 change: 1 addition & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4435,6 +4435,7 @@ static_library("browser") {
"//components/soda:constants",
"//components/ukm/content",
"//components/user_notes:features",
"//components/user_notes:user_notes_prefs",
"//components/user_notes/browser",
"//components/user_notes/interfaces",
"//components/user_notes/storage",
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/prefs/browser_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@
#include "components/live_caption/live_caption_controller.h"
#include "components/live_caption/live_translate_controller.h"
#include "components/ntp_tiles/custom_links_manager_impl.h"
#include "components/user_notes/user_notes_prefs.h"
#endif // BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_CHROMEOS)
Expand Down Expand Up @@ -1486,6 +1487,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry,
UnifiedAutoplayConfig::RegisterProfilePrefs(registry);
CartService::RegisterProfilePrefs(registry);
commerce::ShoppingListUiTabHelper::RegisterProfilePrefs(registry);
user_notes::RegisterProfilePrefs(registry);
#if !BUILDFLAG(IS_CHROMEOS_LACROS)
captions::LiveCaptionController::RegisterProfilePrefs(registry);
#endif // !BUILDFLAG(IS_CHROMEOS_LACROS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface UserNotesApiProxy {
updateNote(guid: string, text: string): Promise<{success: boolean}>;
deleteNote(guid: string): Promise<{success: boolean}>;
noteOverviewSelected(url: Url, clickModifiers: ClickModifiers): void;
setSortOrder(sortByNewest: boolean): void;
getCallbackRouter(): UserNotesPageCallbackRouter;
}

Expand Down Expand Up @@ -61,6 +62,10 @@ export class UserNotesApiProxyImpl implements UserNotesApiProxy {
this.handler.noteOverviewSelected(url, clickModifiers);
}

setSortOrder(sortByNewest: boolean) {
this.handler.setSortOrder(sortByNewest);
}

getCallbackRouter() {
return this.callbackRouter;
}
Expand Down
31 changes: 28 additions & 3 deletions chrome/browser/resources/side_panel/user_notes/user_notes_list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import './user_note.js';

import {CrActionMenuElement} from '//resources/cr_elements/cr_action_menu/cr_action_menu.js';
import {loadTimeData} from '//resources/js/load_time_data.js';
import {assert} from 'chrome://resources/js/assert_ts.js';
import {DomRepeat, DomRepeatEvent, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {Note} from './user_notes.mojom-webui.js';
Expand Down Expand Up @@ -44,7 +45,9 @@ export class UserNotesListElement extends PolymerElement {
activeSortIndex_: {
type: Number,
observer: 'onActiveSortIndexChanged_',
value: 1,
value: function() {
return loadTimeData.getBoolean('sortByNewest') ? 0 : 1;
},
},

sortTypes_: {
Expand All @@ -61,7 +64,28 @@ export class UserNotesListElement extends PolymerElement {
private sortTypes_: string[];
private userNotesApi_: UserNotesApiProxy =
UserNotesApiProxyImpl.getInstance();
private listenerIds_: number[] = [];
private listenerId_: number|null = null;

override connectedCallback() {
super.connectedCallback();

const callbackRouter = this.userNotesApi_.getCallbackRouter();
this.listenerId_ = callbackRouter.sortByNewestPrefChanged.addListener(
(sortByNewest: boolean) => {
const sortIndex = sortByNewest ? 0 : 1;
if (this.activeSortIndex_ !== sortIndex) {
this.activeSortIndex_ = sortIndex;
}
});
}

override disconnectedCallback() {
super.disconnectedCallback();

assert(this.listenerId_);
this.userNotesApi_.getCallbackRouter().removeListener(this.listenerId_);
this.listenerId_ = null;
}

private onAllNotesClick_(event: MouseEvent) {
event.preventDefault();
Expand Down Expand Up @@ -94,7 +118,8 @@ export class UserNotesListElement extends PolymerElement {
event.preventDefault();
event.stopPropagation();
this.$.sortMenu.close();
this.activeSortIndex_ = event.model.index;
const sortByNewest = event.model.index === 0;
this.userNotesApi_.setSortOrder(sortByNewest);
}

private onActiveSortIndexChanged_() {
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1854,6 +1854,7 @@ static_library("ui") {
"//components/user_education/common",
"//components/user_education/webui",
"//components/user_notes:features",
"//components/user_notes:user_notes_prefs",
"//components/vector_icons",
"//components/web_modal",
"//components/webauthn/content/browser",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ interface UserNotesPageHandler {
// should be shown.
NoteOverviewSelected(url.mojom.Url url,
ui.mojom.ClickModifiers click_modifiers);

// Called when the sort option has been updated and the profile pref should be
// updated.
SetSortOrder(bool sort_by_newest);
};

// WebUI-side handler for requests from the browser.
Expand All @@ -98,4 +102,8 @@ interface UserNotesPage {
// This could be triggered by either switching tab or navigating to
// a new URL.
CurrentTabUrlChanged();

// Called when the kUserNotesSortByNewest pref has changed so that the UI can
// match this sort order if it doesn't already.
SortByNewestPrefChanged(bool sort_by_newest);
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
#include "components/power_bookmarks/common/power.h"
#include "components/power_bookmarks/common/power_overview.h"
#include "components/power_bookmarks/core/power_bookmark_service.h"
#include "components/prefs/pref_service.h"
#include "components/sync/protocol/power_bookmark_specifics.pb.h"
#include "components/user_notes/user_notes_prefs.h"
#include "ui/base/l10n/time_format.h"
#include "ui/base/mojom/window_open_disposition.mojom.h"
#include "ui/base/window_open_disposition.h"
Expand Down Expand Up @@ -105,6 +107,11 @@ UserNotesPageHandler::UserNotesPageHandler(
service_(PowerBookmarkServiceFactory::GetForBrowserContext(profile_)),
browser_(browser),
user_notes_ui_(user_notes_ui) {
pref_change_registrar_.Init(profile_->GetPrefs());
pref_change_registrar_.Add(
prefs::kUserNotesSortByNewest,
base::BindRepeating(&UserNotesPageHandler::OnSortByNewestPrefChanged,
base::Unretained(this)));
service_->AddObserver(this);
DCHECK(browser_);
browser_->tab_strip_model()->AddObserver(this);
Expand Down Expand Up @@ -222,6 +229,22 @@ void UserNotesPageHandler::NoteOverviewSelected(
browser_->OpenURL(params);
}

void UserNotesPageHandler::SetSortOrder(bool sort_by_newest) {
PrefService* pref_service = profile_->GetPrefs();
if (pref_service && pref_service->GetBoolean(prefs::kUserNotesSortByNewest) !=
sort_by_newest) {
pref_service->SetBoolean(prefs::kUserNotesSortByNewest, sort_by_newest);
}
}

void UserNotesPageHandler::OnSortByNewestPrefChanged() {
PrefService* pref_service = profile_->GetPrefs();
if (pref_service) {
page_->SortByNewestPrefChanged(
pref_service->GetBoolean(prefs::kUserNotesSortByNewest));
}
}

void UserNotesPageHandler::OnPowersChanged() {
page_->NotesChanged();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "chrome/browser/ui/webui/side_panel/user_notes/user_notes.mojom.h"
#include "components/power_bookmarks/common/power_bookmark_observer.h"
#include "components/power_bookmarks/core/power_bookmark_service.h"
#include "components/prefs/pref_change_registrar.h"
#include "content/public/browser/web_contents_observer.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver.h"
Expand Down Expand Up @@ -55,6 +56,9 @@ class UserNotesPageHandler : public side_panel::mojom::UserNotesPageHandler,
void NoteOverviewSelected(
const ::GURL& url,
ui::mojom::ClickModifiersPtr click_modifiers) override;
void SetSortOrder(bool sort_by_newest) override;

void OnSortByNewestPrefChanged();

void SetCurrentTabUrlForTesting(GURL url) { current_tab_url_ = url; }

Expand All @@ -78,6 +82,7 @@ class UserNotesPageHandler : public side_panel::mojom::UserNotesPageHandler,
mojo::Receiver<side_panel::mojom::UserNotesPageHandler> receiver_;
mojo::Remote<side_panel::mojom::UserNotesPage> page_;
const raw_ptr<Profile> profile_;
PrefChangeRegistrar pref_change_registrar_;
const raw_ptr<power_bookmarks::PowerBookmarkService> service_;
const raw_ptr<Browser> browser_;
raw_ptr<UserNotesSidePanelUI> user_notes_ui_ = nullptr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class MockUserNotesPage : public side_panel::mojom::UserNotesPage {

MOCK_METHOD0(NotesChanged, void());
MOCK_METHOD0(CurrentTabUrlChanged, void());
MOCK_METHOD1(SortByNewestPrefChanged, void(bool));
};

struct Note {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
#include "chrome/grit/side_panel_shared_resources_map.h"
#include "chrome/grit/side_panel_user_notes_resources.h"
#include "chrome/grit/side_panel_user_notes_resources_map.h"
#include "components/prefs/pref_service.h"
#include "components/strings/grit/components_strings.h"
#include "components/user_notes/user_notes_prefs.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui_data_source.h"
#include "ui/base/ui_base_features.h"
Expand Down Expand Up @@ -45,6 +47,13 @@ UserNotesSidePanelUI::UserNotesSidePanelUI(content::WebUI* web_ui)
webui::AddLocalizedString(source, str.name, str.id);
}

Profile* const profile = Profile::FromWebUI(web_ui);
PrefService* pref_service = profile->GetPrefs();
if (pref_service) {
source->AddBoolean("sortByNewest",
pref_service->GetBoolean(prefs::kUserNotesSortByNewest));
}

source->AddString(
"chromeRefresh2023Attribute",
features::IsChromeRefresh2023() ? "chrome-refresh-2023" : "");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class TestUserNotesApiProxy extends TestBrowserProxy implements
'getNoteOverviews',
'newNoteFinished',
'noteOverviewSelected',
'setSortOrder',
'showUi',
'updateNote',
]);
Expand Down Expand Up @@ -56,6 +57,10 @@ export class TestUserNotesApiProxy extends TestBrowserProxy implements
this.methodCalled('noteOverviewSelected', url, clickModifiers);
}

setSortOrder(sortByNewest: boolean) {
this.methodCalled('setSortOrder', sortByNewest);
}

showUi() {
this.methodCalled('showUi');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ suite('UserNotesListTest', () => {
const sortNewestButton =
sortMenu.querySelectorAll('.dropdown-item')[0]! as HTMLButtonElement;
sortNewestButton.click();
const sortByNewest = await testProxy.whenCalled('setSortOrder');
testProxy.getCallbackRouterRemote().sortByNewestPrefChanged(sortByNewest);
await flushTasks();
// Verify note order.
notesElements = queryNotes();
Expand Down
14 changes: 14 additions & 0 deletions components/user_notes/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,17 @@ static_library("features") {

deps = [ "//base" ]
}

static_library("user_notes_prefs") {
sources = [
"user_notes_prefs.cc",
"user_notes_prefs.h",
]

deps = [
":features",
"//base",
"//components/pref_registry:pref_registry",
"//components/prefs",
]
}
2 changes: 2 additions & 0 deletions components/user_notes/DEPS
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
include_rules = [
"+components/keyed_service/core",
"+components/pref_registry",
"+components/prefs",
"+ui",
]
25 changes: 25 additions & 0 deletions components/user_notes/user_notes_prefs.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/user_notes/user_notes_prefs.h"

#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/user_notes/user_notes_features.h"

namespace prefs {

const char kUserNotesSortByNewest[] = "user_notes.sort_by_newest";

} // namespace prefs

namespace user_notes {

void RegisterProfilePrefs(PrefRegistrySimple* registry) {
if (user_notes::IsUserNotesEnabled()) {
registry->RegisterBooleanPref(prefs::kUserNotesSortByNewest, false);
}
}

} // namespace user_notes
25 changes: 25 additions & 0 deletions components/user_notes/user_notes_prefs.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef COMPONENTS_USER_NOTES_USER_NOTES_PREFS_H_
#define COMPONENTS_USER_NOTES_USER_NOTES_PREFS_H_

class PrefRegistrySimple;

namespace prefs {

// Boolean that stores whether user notes should be sorted by newest and is used
// for the sort order to display user's notes.
extern const char kUserNotesSortByNewest[];

} // namespace prefs

namespace user_notes {

// Registers user preferences related to user notes.
void RegisterProfilePrefs(PrefRegistrySimple* registry);

} // namespace user_notes

#endif // COMPONENTS_USER_NOTES_USER_NOTES_PREFS_H_

0 comments on commit 065085a

Please sign in to comment.