Paths

Table of Contentst

-
native/profile/
-
profile/
1
relationship-list-item.react.js

[native] make user profiles accessible from RelationshipListItem
ClosedPublic
Actions

Authored by ginsu on Sep 25 2023, 7:04 AM.

Details

Reviewers

atul
inka

Commits

rCOMM0900e7074de9: [native] make user profiles accessible from RelationshipListItem

Summary

This diff adds a TouchableOpacity to the RelationshipListItem component and navigates to the user profile bottom sheet whenever the onPress is triggered

Depends on D9285

Test Plan

Please see the demo video below

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Sep 25 2023, 7:04 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptSep 25 2023, 7:04 AM

ginsu edited the summary of this revision. (Show Details)Sep 25 2023, 7:11 AM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, inka.

ginsu added a parent revision: D9285: [native] simplify RelationshipListItem jsx.

ginsu added inline comments.Sep 25 2023, 7:15 AM

native/profile/relationship-list-item.react.js
172 ↗	(On Diff #31400)	The red background shows the `TouchableOpacity`: I think it would be a be a better user experience if we had the entire item be a touchable instead of just the avatar/username

ginsu attached a referenced file: F773403: Screenshot 2023-09-25 at 10.10.09 AM.png. (Show Details)Sep 25 2023, 7:15 AM

Harbormaster completed remote builds in B22829: Diff 31400.Sep 25 2023, 7:22 AM

ginsu requested review of this revision.Sep 25 2023, 7:22 AM

ginsu added a child revision: D9289: [native] make user profile accessible from message author avatar.Sep 25 2023, 7:23 AM

atul accepted this revision.Sep 26 2023, 12:54 PM

atul added a subscriber: • ted.

atul added inline comments.

native/profile/relationship-list-item.react.js
172 ↗	(On Diff #31400)	What surfaces should be "touchable" is probably something we should consider as part of the Design System (at some point) CC @ashoat @ted In this particular case, it feels a little weird that we're extending so far to the right, but on the left we go just until the left edge. I guess this is a new scenario, because previously we were just listing "static" avatars/usernames and left all the interaction to the right-justified icon. Unsolicited, but if it were up to me I think it'd make sense to give the avatar/username some sort of inset to make it clear that A. the element is interactive B. the bounds of the tap target Something like: or

This revision is now accepted and ready to land.Sep 26 2023, 12:54 PM

atul attached a referenced file: F774976: 1181e5.png. (Show Details)Sep 26 2023, 12:54 PM

atul attached a referenced file: F774975: 684a67.png. (Show Details)

ashoat added inline comments.Sep 27 2023, 6:10 AM

native/profile/relationship-list-item.react.js
172 ↗	(On Diff #31400)	I like @atul's idea. Curious for @ted's thoughts, both on Atul's idea and also on how we can include touchable surface / interaction design of touchables in the design system

• ted added inline comments.Sep 27 2023, 10:59 AM

native/profile/relationship-list-item.react.js
172 ↗	(On Diff #31400)	In this particular case, it feels a little weird that we're extending so far to the right, but on the left we go just until the left edge. I think the touch point can extend more on the left, to where `Search` starts at the input field above. @atul @ashoat I think adding touch points to the design system makes perfect sense and can be included for both web and mobile components. Each design system component lives on its own page in Figma, which will also include small documents to specifically point out the interactive touch points. These documents can point out other component rules such as places to use this component, certain rules of how to use this component, etc. For the concepts you shared, I don't think that adding an inset is necessary to point out that the element is interactive. Adding an inset feels a bit cluttered to the overall list layout. If we do want to use an inset container for the avatar and username, then reducing the separator lines (smaller, more subtle) will help with over-cluttering. I took a quick look at Discord's username touchpoints in their list. In the video, I am tapping as far right as I can before the two action buttons. The entire width seems tappable except for the two buttons. Not saying we need to do exactly how Discord handles this interaction, I wanted to see how other apps were handling this. With that, I think the UI is fine how it is, but always open if more people think we should add an inset or interactive indicator. Thank you!

• ted attached a referenced file: F776853: RPReplay_Final1695836467.MP4. (Show Details)Sep 27 2023, 10:59 AM

ginsu added inline comments.Sep 27 2023, 11:06 AM

native/profile/relationship-list-item.react.js
172 ↗	(On Diff #31400)	For the concepts you shared, I don't think that adding an inset is necessary to point out that the element is interactive. Adding an inset feels a bit cluttered to the overall list layout. If we do want to use an inset container for the avatar and username, then reducing the separator lines (smaller, more subtle) will help with over-cluttering. I agree with this point, I think especially since we have the edit (pencil) icon to the right it would feel strange if that didn't have an inset/obvious indicator that the icon is touchable but the avatar/username does. To polish this up, I can make the entire container a touchable or at the very least, also extend the touchable to be touching the left side of this container

address feedback

Harbormaster completed remote builds in B23042: Diff 31731.Oct 6 2023, 4:09 AM

rebase before landing

Harbormaster completed remote builds in B23157: Diff 31888.Oct 11 2023, 1:48 AM

Closed by commit rCOMM0900e7074de9: [native] make user profiles accessible from RelationshipListItem (authored by ginsu). · Explain WhyOct 11 2023, 2:38 AM

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMM0900e7074de9: [native] make user profiles accessible from RelationshipListItem.

Revision Contents
Changeset List

Path

Size

native/

profile/

relationship-list-item.react.js

17 lines

Diff 31731

View Options

native/profile/relationship-list-item.react.js

Show First 20 Lines • Show All 487 Lines • ▼ Show 20 Lines	attempt_delete_file(
"Failed to delete unencrypted database.");		"Failed to delete unencrypted database.");
attempt_rename_file(		attempt_rename_file(
temp_encrypted_db_path,		temp_encrypted_db_path,
SQLiteQueryExecutor::sqliteFilePath,		SQLiteQueryExecutor::sqliteFilePath,
"Failed to move encrypted database to default location.");		"Failed to move encrypted database to default location.");
Logger::log("Encryption completed successfully.");		Logger::log("Encryption completed successfully.");
}		}

		auto &SQLiteQueryExecutor::getStorage() {
		static auto storage = make_storage(
		SQLiteQueryExecutor::sqliteFilePath,
		make_index("messages_idx_thread_time", &Message::thread, &Message::time),
		make_index("media_idx_container", &Media::container),
		make_table(
		"drafts",
		make_column("key", &Draft::key, unique(), primary_key()),
		make_column("text", &Draft::text)),
		make_table(
		"messages",
		make_column("id", &Message::id, unique(), primary_key()),
		make_column("local_id", &Message::local_id),
		make_column("thread", &Message::thread),
		make_column("user", &Message::user),
		make_column("type", &Message::type),
		make_column("future_type", &Message::future_type),
		make_column("content", &Message::content),
		make_column("time", &Message::time)),
		make_table(
		"olm_persist_account",
		make_column("id", &OlmPersistAccount::id, unique(), primary_key()),
		make_column("account_data", &OlmPersistAccount::account_data)),
		make_table(
		"olm_persist_sessions",
		make_column(
		"target_user_id",
		&OlmPersistSession::target_user_id,
		unique(),
		primary_key()),
		make_column("session_data", &OlmPersistSession::session_data)),
		make_table(
		"media",
		make_column("id", &Media::id, unique(), primary_key()),
		make_column("container", &Media::container),
		make_column("thread", &Media::thread),
		make_column("uri", &Media::uri),
		make_column("type", &Media::type),
		make_column("extras", &Media::extras)),
		make_table(
		"threads",
		make_column("id", &Thread::id, unique(), primary_key()),
		make_column("type", &Thread::type),
		make_column("name", &Thread::name),
		make_column("description", &Thread::description),
		make_column("color", &Thread::color),
		make_column("creation_time", &Thread::creation_time),
		make_column("parent_thread_id", &Thread::parent_thread_id),
		make_column("containing_thread_id", &Thread::containing_thread_id),
		make_column("community", &Thread::community),
		make_column("members", &Thread::members),
		make_column("roles", &Thread::roles),
		make_column("current_user", &Thread::current_user),
		make_column("source_message_id", &Thread::source_message_id),
		make_column("replies_count", &Thread::replies_count)),
		make_table(
		"metadata",
		make_column("name", &Metadata::name, unique(), primary_key()),
		make_column("data", &Metadata::data)));
		storage.on_open = on_database_open;
		return storage;
		}

typedef bool ShouldBeInTransaction;		typedef bool ShouldBeInTransaction;
typedef std::pair<std::function<bool(sqlite3 *)>, ShouldBeInTransaction>		typedef std::pair<std::function<bool(sqlite3 *)>, ShouldBeInTransaction>
SQLiteMigration;		SQLiteMigration;
std::vector<std::pair<uint, SQLiteMigration>> migrations{		std::vector<std::pair<uint, SQLiteMigration>> migrations{
{{1, {create_drafts_table, true}},		{{1, {create_drafts_table, true}},
{2, {rename_threadID_to_key, true}},		{2, {rename_threadID_to_key, true}},
{4, {create_persist_account_table, true}},		{4, {create_persist_account_table, true}},
{5, {create_persist_sessions_table, true}},		{5, {create_persist_sessions_table, true}},
Show All 28 Lines	void SQLiteQueryExecutor::migrate() const {

int current_user_version = sqlite3_column_int(user_version_stmt, 0);		int current_user_version = sqlite3_column_int(user_version_stmt, 0);
sqlite3_finalize(user_version_stmt);		sqlite3_finalize(user_version_stmt);

std::stringstream version_msg;		std::stringstream version_msg;
version_msg << "db version: " << current_user_version << std::endl;		version_msg << "db version: " << current_user_version << std::endl;
Logger::log(version_msg.str());		Logger::log(version_msg.str());

		if (current_user_version == 0) {
		SQLiteQueryExecutor::getStorage().sync_schema();

		Logger::log("Creating new database, syncing structure with ORM storage.");

		auto latest_version = migrations.back().first;

		std::stringstream update_version;
		update_version << "PRAGMA user_version=" << latest_version << ";";
		auto update_version_str = update_version.str();

		char *error;
		sqlite3_exec(db, update_version_str.c_str(), nullptr, nullptr, &error);

		if (error) {
		std::ostringstream errorStream;
		errorStream << "Error saving database version: " << error;
		Logger::log(errorStream.str());
		marcinUnsubmitted Not Done Inline Actions How is application going to function if this error occurs? In most places we the runtime error to kill the app if database operation fails since application is not going to work correctly. I just want to know whether it is also the case here. marcin: How is application going to function if this error occurs? In most places we the runtime error…
		sqlite3_free(error);
		}

		return;
		}

for (const auto &[idx, migration] : migrations) {		for (const auto &[idx, migration] : migrations) {
if (idx <= current_user_version) {		if (idx <= current_user_version) {
continue;		continue;
}		}
const auto &[applyMigration, shouldBeInTransaction] = migration;		const auto &[applyMigration, shouldBeInTransaction] = migration;

std::stringstream migration_msg;		std::stringstream migration_msg;

Show All 28 Lines	void SQLiteQueryExecutor::assign_encryption_key() {
CommSecureStore commSecureStore{};		CommSecureStore commSecureStore{};
std::string encryptionKey = comm::crypto::Tools::generateRandomHexString(		std::string encryptionKey = comm::crypto::Tools::generateRandomHexString(
SQLiteQueryExecutor::sqlcipherEncryptionKeySize);		SQLiteQueryExecutor::sqlcipherEncryptionKeySize);
commSecureStore.set(		commSecureStore.set(
SQLiteQueryExecutor::secureStoreEncryptionKeyID, encryptionKey);		SQLiteQueryExecutor::secureStoreEncryptionKeyID, encryptionKey);
SQLiteQueryExecutor::encryptionKey = encryptionKey;		SQLiteQueryExecutor::encryptionKey = encryptionKey;
}		}

auto &SQLiteQueryExecutor::getStorage() {
static auto storage = make_storage(
SQLiteQueryExecutor::sqliteFilePath,
make_index("messages_idx_thread_time", &Message::thread, &Message::time),
make_index("media_idx_container", &Media::container),
make_table(
"drafts",
make_column("key", &Draft::key, unique(), primary_key()),
make_column("text", &Draft::text)),
make_table(
"messages",
make_column("id", &Message::id, unique(), primary_key()),
make_column("local_id", &Message::local_id),
make_column("thread", &Message::thread),
make_column("user", &Message::user),
make_column("type", &Message::type),
make_column("future_type", &Message::future_type),
make_column("content", &Message::content),
make_column("time", &Message::time)),
make_table(
"olm_persist_account",
make_column("id", &OlmPersistAccount::id, unique(), primary_key()),
make_column("account_data", &OlmPersistAccount::account_data)),
make_table(
"olm_persist_sessions",
make_column(
"target_user_id",
&OlmPersistSession::target_user_id,
unique(),
primary_key()),
make_column("session_data", &OlmPersistSession::session_data)),
make_table(
"media",
make_column("id", &Media::id, unique(), primary_key()),
make_column("container", &Media::container),
make_column("thread", &Media::thread),
make_column("uri", &Media::uri),
make_column("type", &Media::type),
make_column("extras", &Media::extras)),
make_table(
"threads",
make_column("id", &Thread::id, unique(), primary_key()),
make_column("type", &Thread::type),
make_column("name", &Thread::name),
make_column("description", &Thread::description),
make_column("color", &Thread::color),
make_column("creation_time", &Thread::creation_time),
make_column("parent_thread_id", &Thread::parent_thread_id),
make_column("containing_thread_id", &Thread::containing_thread_id),
make_column("community", &Thread::community),
make_column("members", &Thread::members),
make_column("roles", &Thread::roles),
make_column("current_user", &Thread::current_user),
make_column("source_message_id", &Thread::source_message_id),
make_column("replies_count", &Thread::replies_count)),
make_table(
"metadata",
make_column("name", &Metadata::name, unique(), primary_key()),
make_column("data", &Metadata::data)));
storage.on_open = on_database_open;
return storage;
}

void SQLiteQueryExecutor::initialize(std::string &databasePath) {		void SQLiteQueryExecutor::initialize(std::string &databasePath) {
std::call_once(SQLiteQueryExecutor::initialized, [&databasePath]() {		std::call_once(SQLiteQueryExecutor::initialized, [&databasePath]() {
SQLiteQueryExecutor::sqliteFilePath = databasePath;		SQLiteQueryExecutor::sqliteFilePath = databasePath;
CommSecureStore commSecureStore{};		CommSecureStore commSecureStore{};
folly::Optional<std::string> maybeEncryptionKey =		folly::Optional<std::string> maybeEncryptionKey =
commSecureStore.get(SQLiteQueryExecutor::secureStoreEncryptionKeyID);		commSecureStore.get(SQLiteQueryExecutor::secureStoreEncryptionKeyID);

if (file_exists(databasePath) && maybeEncryptionKey) {		if (file_exists(databasePath) && maybeEncryptionKey) {
▲ Show 20 Lines • Show All 277 Lines • Show Last 20 Lines

[native] make user profiles accessible from RelationshipListItemClosedPublicActions