Page MenuHomePhabricator
Differential D9750

[lib] Refactor keyserver reducer pt.1
ClosedPublic
Actions

Authored by inka on Nov 7 2023, 7:51 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 23, 12:55 PM
Unknown Object (File)
Wed, Mar 19, 6:38 AM
Unknown Object (File)
Tue, Mar 18, 12:34 PM
Unknown Object (File)
Tue, Mar 18, 12:34 PM
Unknown Object (File)
Tue, Mar 18, 12:33 PM
Unknown Object (File)
Tue, Mar 18, 12:33 PM
Unknown Object (File)
Tue, Mar 18, 12:33 PM
Unknown Object (File)
Tue, Mar 18, 12:24 PM
View All Files
Subscribers
ashoat
tomek
will

Details

Reviewers
michal
kamil
ginsu
Commits
rCOMMa00dfd9af946: [lib] Refactor keyserver reducer pt.1
Summary

issue: https://linear.app/comm/issue/ENG-4729/refactor-reducers
In this diff I refactored the firt two ifs of this reducer. The rest of the logic will be refactored in next diffs

Test Plan

Dispatched setNewSessionActionType and resetUserStateActionType, chceked that the reducer works as expected for them

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka created this revision.Nov 7 2023, 7:51 AM
Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptNov 7 2023, 7:51 AM
inka added a child revision: D9763: [lib] Inline reduceUpdatesCurrentAsOf.Nov 8 2023, 6:18 AM
inka published this revision for review.Nov 8 2023, 6:20 AM
Harbormaster completed remote builds in B23870: Diff 32899.Nov 8 2023, 1:16 PM
inka mentioned this in D9664: [lib] add conditions for adding keyserver to the keyserver store.Nov 9 2023, 3:38 AM
ginsu mentioned this in D9829: [lib] introduce removeKeyserverActionType condition to keyserver reducer.Nov 10 2023, 11:59 AM
inka updated this revision to Diff 33199.Nov 14 2023, 12:56 AM

Rebase

inka added inline comments.Nov 14 2023, 12:58 AM
lib/reducers/keyserver-reducer.js
30–40 ↗(On Diff #33199)

Will be updated in D9815

Harbormaster completed remote builds in B24099: Diff 33199.Nov 14 2023, 1:15 AM
michal accepted this revision.Nov 14 2023, 4:06 AM
michal added inline comments.
lib/types/session-types.js
96–102 ↗(On Diff #33199)

Can we update this type to read-only?

This revision is now accepted and ready to land.Nov 14 2023, 4:06 AM
inka updated this revision to Diff 33213.Nov 14 2023, 7:16 AM

Update SetSessionPayload to be read only

Harbormaster completed remote builds in B24113: Diff 33213.Nov 14 2023, 7:35 AM
ginsu mentioned this in rCOMMb89136d8d288: [lib] introduce removeKeyserverActionType condition to keyserver reducer.Nov 14 2023, 5:49 PM
Closed by commit rCOMMa00dfd9af946: [lib] Refactor keyserver reducer pt.1 (authored by inka). · Explain WhyNov 22 2023, 7:24 AM
This revision was automatically updated to reflect the committed changes.
inka added a commit: rCOMMa00dfd9af946: [lib] Refactor keyserver reducer pt.1.

Revision Contents
Changeset List

PathSize
lib/
reducers/
26 lines
socket/
1 line
types/
9 lines
utils/
19 lines

Diff 33213

View Options

lib/reducers/keyserver-reducer.js

Show First 20 Lines • Show All 130 Lines • ▼ Show 20 Lines
const styles = StyleSheet.create({ const styles = StyleSheet.create({
dummyMessage: { dummyMessage: {
paddingHorizontal: 12, paddingHorizontal: 12,
paddingVertical: 6, paddingVertical: 6,
}, },
message: { message: {
overflow: 'hidden', overflow: 'hidden',
paddingHorizontal: 12, paddingHorizontal: 12,
paddingVertical: 6, paddingVertical: 8,
}, },
text: { text: {
fontFamily: 'Arial', fontFamily: 'Arial',
fontSize: 18, fontSize: 14,
benschacAuthorUnsubmitted
Done
Inline Actions

I actually think this is a bit too small. But that's what's in the design.

https://www.figma.com/file/L675ETKDnGaSwlpZAw4MIC/Mobile-App?node-id=351%3A455

@ashoat - I think 16px looks better.

image.png (1×2 px, 2 MB)

benschac: I actually think this is a bit too small. But that's what's in the design. https://www.figma.
lineHeight: 22,
}, },
}); });
export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement }; export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement };
View Options

lib/socket/socket.react.js

Show First 20 Lines • Show All 130 Lines • ▼ Show 20 Lines
const styles = StyleSheet.create({ const styles = StyleSheet.create({
dummyMessage: { dummyMessage: {
paddingHorizontal: 12, paddingHorizontal: 12,
paddingVertical: 6, paddingVertical: 6,
}, },
message: { message: {
overflow: 'hidden', overflow: 'hidden',
paddingHorizontal: 12, paddingHorizontal: 12,
paddingVertical: 6, paddingVertical: 8,
}, },
text: { text: {
fontFamily: 'Arial', fontFamily: 'Arial',
fontSize: 18, fontSize: 14,
benschacAuthorUnsubmitted
Done
Inline Actions

I actually think this is a bit too small. But that's what's in the design.

https://www.figma.com/file/L675ETKDnGaSwlpZAw4MIC/Mobile-App?node-id=351%3A455

@ashoat - I think 16px looks better.

image.png (1×2 px, 2 MB)

benschac: I actually think this is a bit too small. But that's what's in the design. https://www.figma.
lineHeight: 22,
}, },
}); });
export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement }; export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement };
View Options

lib/types/session-types.js

Show First 20 Lines • Show All 130 Lines • ▼ Show 20 Lines
const styles = StyleSheet.create({ const styles = StyleSheet.create({
dummyMessage: { dummyMessage: {
paddingHorizontal: 12, paddingHorizontal: 12,
paddingVertical: 6, paddingVertical: 6,
}, },
message: { message: {
overflow: 'hidden', overflow: 'hidden',
paddingHorizontal: 12, paddingHorizontal: 12,
paddingVertical: 6, paddingVertical: 8,
}, },
text: { text: {
fontFamily: 'Arial', fontFamily: 'Arial',
fontSize: 18, fontSize: 14,
benschacAuthorUnsubmitted
Done
Inline Actions

I actually think this is a bit too small. But that's what's in the design.

https://www.figma.com/file/L675ETKDnGaSwlpZAw4MIC/Mobile-App?node-id=351%3A455

@ashoat - I think 16px looks better.

image.png (1×2 px, 2 MB)

benschac: I actually think this is a bit too small. But that's what's in the design. https://www.figma.
lineHeight: 22,
}, },
}); });
export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement }; export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement };
View Options

lib/utils/action-utils.js

Show First 20 Lines • Show All 130 Lines • ▼ Show 20 Lines
const styles = StyleSheet.create({ const styles = StyleSheet.create({
dummyMessage: { dummyMessage: {
paddingHorizontal: 12, paddingHorizontal: 12,
paddingVertical: 6, paddingVertical: 6,
}, },
message: { message: {
overflow: 'hidden', overflow: 'hidden',
paddingHorizontal: 12, paddingHorizontal: 12,
paddingVertical: 6, paddingVertical: 8,
}, },
text: { text: {
fontFamily: 'Arial', fontFamily: 'Arial',
fontSize: 18, fontSize: 14,
benschacAuthorUnsubmitted
Done
Inline Actions

I actually think this is a bit too small. But that's what's in the design.

https://www.figma.com/file/L675ETKDnGaSwlpZAw4MIC/Mobile-App?node-id=351%3A455

@ashoat - I think 16px looks better.

image.png (1×2 px, 2 MB)

benschac: I actually think this is a bit too small. But that's what's in the design. https://www.figma.
lineHeight: 22,
}, },
}); });
export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement }; export { InnerTextMessage, dummyNodeForTextMessageHeightMeasurement };