Page MenuHomePhabricator
Differential D8924

[keyserver][lib][native] Add connection to keyserverStore
ClosedPublic
Actions

Authored by inka on Aug 23 2023, 5:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 3, 11:38 AM
Unknown Object (File)
Thu, Apr 3, 5:08 AM
Unknown Object (File)
Thu, Apr 3, 2:05 AM
Unknown Object (File)
Wed, Apr 2, 7:13 AM
Unknown Object (File)
Mar 1 2025, 3:25 PM
Unknown Object (File)
Mar 1 2025, 3:25 PM
Unknown Object (File)
Mar 1 2025, 3:23 PM
Unknown Object (File)
Mar 1 2025, 3:00 PM
View All 78 Files
Subscribers
ashoat
tomek

Details

Reviewers
michal
kamil
Commits
rCOMM22205172816b: [keyserver][lib][native] Add connection to keyserverStore
Summary

issue: https://linear.app/comm/issue/ENG-4460/refactor-connection-field
First step is to add connection field to keyserverStore

Test Plan

ran yarn flow-all

Diff Detail

Repository
rCOMM Comm
Branch
inka/redux3
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka created this revision.Aug 23 2023, 5:21 AM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptAug 23 2023, 5:21 AM
Harbormaster completed remote builds in B22035: Diff 30251.Aug 23 2023, 5:39 AM
inka requested review of this revision.Aug 23 2023, 5:39 AM
inka added a child revision: D8931: [lib][native][web] Use connection from keyserverStore.Aug 23 2023, 9:00 AM
michal accepted this revision.Aug 23 2023, 10:45 PM
This revision is now accepted and ready to land.Aug 23 2023, 10:45 PM
Closed by commit rCOMM22205172816b: [keyserver][lib][native] Add connection to keyserverStore (authored by inka). · Explain WhyAug 25 2023, 2:25 AM
This revision was automatically updated to reflect the committed changes.
inka added a commit: rCOMM22205172816b: [keyserver][lib][native] Add connection to keyserverStore.
inka mentioned this in D8950: Revert connction field refactor.Aug 25 2023, 7:47 AM
inka mentioned this in rCOMM633e6c16e4fa: Revert connction field refactor.Aug 25 2023, 8:40 AM
ashoat added inline comments.Sep 7 2023, 12:57 PM
keyserver/src/responders/website-responders.js
511 ↗(On Diff #30302)

We should avoid await constructions like this. await keywords should appear at the start of a statement, or at the start of the right-hand side of an assignment. await is a very powerful keyword and we should always aim to make it very clear where it appears, instead of sneaking it into declarations like this.

const connectionPromise = (async () => {
  const actualizedCalendarQuery = await calendarQueryPromise;
  return {
    ...defaultConnectionInfo(viewer.platform ?? 'web', viewer.timeZone),
    actualizedCalendarQuery,
  };
})();

Revision Contents
Changeset List

PathSize
keyserver/
src/
responders/
14 lines
lib/
reducers/
24 lines
types/
4 lines
native/
redux/
1 line

Diff 30251

View Options

keyserver/src/responders/website-responders.js

  • This file was added.
div.containerViewerLiked {
padding: 4px;
border-radius: 4px;
background-color: var(--shades-black-80);
}
div.container:hover {
cursor: pointer;
}
tomekUnsubmitted
Not Done
Inline Actions

For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so?

tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
ginsuAuthorUnsubmitted
Done
Inline Actions

I believe the other icons don't need to set this style because it is built into IcomoonReact. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff

ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
tomekUnsubmitted
Not Done
Inline Actions

It wouldn't make too much sense for Icomoon to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in message-tooltip.css

div.messageActionButtons svg:hover {
  cursor: pointer;
  color: var(--fg);
}

so it should be enough to apply this style also to the new button.

tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
ginsuAuthorUnsubmitted
Done
Inline Actions

Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes
View Options

lib/reducers/keyserver-reducer.js

  • This file was added.
div.containerViewerLiked {
padding: 4px;
border-radius: 4px;
background-color: var(--shades-black-80);
}
div.container:hover {
cursor: pointer;
}
tomekUnsubmitted
Not Done
Inline Actions

For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so?

tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
ginsuAuthorUnsubmitted
Done
Inline Actions

I believe the other icons don't need to set this style because it is built into IcomoonReact. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff

ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
tomekUnsubmitted
Not Done
Inline Actions

It wouldn't make too much sense for Icomoon to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in message-tooltip.css

div.messageActionButtons svg:hover {
  cursor: pointer;
  color: var(--fg);
}

so it should be enough to apply this style also to the new button.

tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
ginsuAuthorUnsubmitted
Done
Inline Actions

Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes
View Options

lib/types/keyserver-types.js

  • This file was added.
div.containerViewerLiked {
padding: 4px;
border-radius: 4px;
background-color: var(--shades-black-80);
}
div.container:hover {
cursor: pointer;
}
tomekUnsubmitted
Not Done
Inline Actions

For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so?

tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
ginsuAuthorUnsubmitted
Done
Inline Actions

I believe the other icons don't need to set this style because it is built into IcomoonReact. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff

ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
tomekUnsubmitted
Not Done
Inline Actions

It wouldn't make too much sense for Icomoon to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in message-tooltip.css

div.messageActionButtons svg:hover {
  cursor: pointer;
  color: var(--fg);
}

so it should be enough to apply this style also to the new button.

tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
ginsuAuthorUnsubmitted
Done
Inline Actions

Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes
View Options

native/redux/redux-setup.js

  • This file was added.
div.containerViewerLiked {
padding: 4px;
border-radius: 4px;
background-color: var(--shades-black-80);
}
div.container:hover {
cursor: pointer;
}
tomekUnsubmitted
Not Done
Inline Actions

For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so?

tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
ginsuAuthorUnsubmitted
Done
Inline Actions

I believe the other icons don't need to set this style because it is built into IcomoonReact. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff

ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
tomekUnsubmitted
Not Done
Inline Actions

It wouldn't make too much sense for Icomoon to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in message-tooltip.css

div.messageActionButtons svg:hover {
  cursor: pointer;
  color: var(--fg);
}

so it should be enough to apply this style also to the new button.

tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
ginsuAuthorUnsubmitted
Done
Inline Actions

Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes