-
-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Axolotl presage #985
Axolotl presage #985
Conversation
Wow! This is huge, and a very big step in the good direction! Thank you for all the hard work! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I couldn't help myself to start reviewing.
I will review further, when you request it.
@@ -23,26 +23,26 @@ | |||
<div v-for="chat in chatList" :key="chat.id" class="row"> | |||
<!-- chat entry --> | |||
<div | |||
:id="chat.ID" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes like these happen, because there is no defined model for objects send to the frontend.
I want to suggest, that it may help to decouple the object models for communication with the frontend (DTO's) from the backend logic. This would introduce a mapping between backend objects and DTO's, but I think its worth it. What's your take on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have an example for how this could be done? The changes here are happening because in rust a variable shouldn't start with an uppercase letter.
axolotl-web/src/pages/ChatList.vue
Outdated
alt="Avatar image" | ||
@error="onImageError($event)" | ||
/> | ||
{{ chat.ID[0] }} | ||
{{ `${chat.title[0]}${chat.title[1]?chat.title[1]:""}` }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to introduce a function for this logic. I'm not a fan of logic within the template.
@@ -43,11 +43,11 @@ | |||
alt="Avatar image" | |||
@error="onImageError($event)" | |||
/> | |||
{{ c.Name[0] ? c.Name[0] + c.Name[1] : c.Name[0] }} | |||
{{ c.name[0] ? c.name[0] + c.name[1] : c.name[0] }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line still contains a bug. Correct would be:
{{ c.name[1] ? c.name[0] + c.name[1] : c.name[0] }}
Also: I would like to introduce a function for this logic. I'm not a fan of logic within the template.
@@ -76,12 +76,12 @@ | |||
alt="Avatar image" | |||
@error="onImageError($event)" | |||
/> | |||
{{ c.Name[0] ? c.Name[0] + c.Name[1] : c.Name[0] }} | |||
{{ c.name[0] ? c.name[0] + c.name[1] : c.name[0] }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm even less happy about copied (wrong) logic within the template.
/* | ||
.avatar{ | ||
border-radius: 50px; | ||
background-color: #2090ea; | ||
width: 50px; | ||
height: 50px; | ||
justify-content: center; | ||
align-items: center; | ||
display: flex; | ||
color: #FFF; | ||
margin:20px; | ||
} */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove it or use it.
" | ||
class="avatar" | ||
> | ||
<div :key="message.ID" :class="{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
message.ID
vs chat.id
can we make this consistent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The message has no field ID anymore, this is still a clean up in progress
<div :key="message.ID" :class="{ | ||
'col-12': true, | ||
'message-container': true, | ||
outgoing: message.is_outgoing, | ||
sent: message.IsSent && message.is_outgoing, | ||
read: message.IsRead && message.is_outgoing, | ||
delivered: message.Receipt && message.is_outgoing, | ||
incoming: !message.is_outgoing, | ||
status: | ||
(message.Flags > 0 && | ||
message.Flags !== 11 && | ||
message.Flags !== 13 && | ||
message.Flags !== 14) || | ||
message.StatusMessage || | ||
(message.Attachment && | ||
message.Attachment.includes('null') && | ||
message.Message === ''), | ||
hidden: message.Flags === 18, | ||
error: message.timestamp === 0 || message.SendingError, | ||
'group-message': | ||
isGroup && (message.Flags === 0 || message.Flags === 12 || message.Flags === 13), | ||
}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a lot of logic here for setting the right css classes. Is there a cleaner way to make this?
And can we move the logic into funktions, if its more than one logic operation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for here, the Flags field doesn't exist anymore but we have to add something like this again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And i agree to move it into functions
isGroup && | ||
(message.Flags === 0 || message.Flags === 12 || message.Flags === 13) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is copied within 5 lines of code.
@@ -93,7 +93,7 @@ | |||
@long-press="paste" | |||
/> | |||
</div> | |||
<div v-if="messageInput !== ''" class="messageInput-btn-container"> | |||
<div v-if="messageInput !== '' || true" class="messageInput-btn-container"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this, as soon as its not needed anymore
9151992
to
b9d47a0
Compare
Add the ability to send text messages
@Blackoverflow thank you for your review! |
Yes, I noticed that it was working in CI and I've copied the steps there as closely as possible. I've tried several checkouts of this branch. Last commit I tried was d30193b |
I figured it out. I was not copying the |
WIP: Avoid npm nodejs version conflict in Clickable images
No problem! It's still a lot to do. |
Hey guys, glad to see you chippin' away at this! Progress is looking good. I'm stoked! Keep at it, you got something awesome here, I can tell!! |
Hi dear axolotl team, first of all thank you for creating axolotl and (more importantly) maintaining it. I have been using it for almost two years now (dispite the frequent changes on the Signal server side). I have a uncommon proposal, but let me explain: My impression is that axolotl has become (at least for me) a "single point of failure" for the whole idea of using an operating system on a mobile phone which is neither Android nor iOS. That is: Axolotl is the only available Signal client for free and open source operating systems. However, there is currently no version that is compatible with the Signal servers so the only option is to use Signal Desktop on a Laptop. I could myself offer the following during my Christmas Holidays (on this branch or on "main")
Let me know if (and how) I can support! |
Remove conflicting yarn.lock
Fix Flatpak build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea why the registration is broken ? I know thhat the server return 422 and I don't mind debugging, but if anyone has knowledge that saves time, it will be appreciated. thanks
This pr replaces the go backend, crayfish, textsecure and crayfish with presage, libsignal-service-rs and libsignal.
The new backend is written in rust instead of go in order to the upstream library from signal and share efforts with whisperfish from sailfishos, flare as gtk app, and gurk-rs a terminal client. The work in progress only supports linking as secondary device for now and after a successful linking axolotl has to be restarted.
There are 2 features for now:
Development
cd axolotl-web && npm i
cargo run --features tauri
cargo tauri dev --features tauri
(this commands enables hot reloading of everything)cargo run --features ut
clickable build --libs axolotlweb && clickable desktop
cargo run -- -d
cd axolotl-web && npm run dev
A lot of things have to be done
In libsignal-service-rs (means broken in all projects)
In presage
Axolotl
Axolotl packaging
We can also add testing