pleroma/pleroma-fe
2
2
Fork 4
Code Issues 344 Pull requests 27 Projects Releases Packages Wiki Activity

Chats #2286

Closed
eugenijm wants to merge 0 commits from gitlab-mr-iid-1019 into develop
pull from: gitlab-mr-iid-1019
merge into: pleroma:develop
Conversation 417 Commits - Files changed -
eugenijm commented 2019-12-03 02:43:13 +00:00
Member
Copy link

Closes #201

Timeline

  • the timeline updates the unread indicator and the message content (video, screenshot)

Starting a new chat

  • allows selecting a user using the search (the default is the people the user most recently chatted with) (video)
  • if the conversation with the selected user already exists, it redirects to it

The chat view

Misc

  • the "Message" button is added to the user profile card, it takes to the latest chat with the user or starts a new one
Closes #201 Timeline - [x] the timeline updates the unread indicator and the message content ([video](/attachments/4a95db10-6d44-4899-9c57-cf754b069239), [screenshot](/attachments/bdd83c81-f6bd-4131-833c-b9e132913918)) Starting a new chat - [x] allows selecting a user using the search (the default is the people the user most recently chatted with) ([video](/attachments/19978698-274c-4521-bff8-295a25638f76)) - [x] if the conversation with the selected user already exists, it redirects to it The chat view - [x] [deletion](/attachments/dee9e01f-5947-4d5d-b456-ddbd53f7fcb9) (the menu appears on message click — works the same way on mobile and desktop) - [x] message sequence by the same user in a 1-1 chat ([screenshot](/attachments/e55a94b9-76cd-454c-a885-c1bd2f74c450)) - [x] mobile layout ([screenshot](/attachments/d1f59abb-3e19-4d8a-a9c0-debbe9aa56c4)) - [x] the colors can be customized at Settings -> Themes -> Advanced -> Chats ([screenshot](/attachments/524521ee-876d-44b3-81fc-b179d9befa35)) - [x] theme support: - [Pleroma Dark](/attachments/64e28cc8-9f0f-44ff-9290-6b4a3e3cc15d) - [Pleroma Light](/attachments/a82b89e5-a38b-4abe-a92d-83d4a12cb2d3) - [Bird](/attachments/6d7b0786-3861-426c-b3eb-52370d5e303a) - [Monokai](/attachments/6ddefbb0-a012-450b-999f-aaf310fe31f8) - [Mammal](/attachments/fe51e664-88f3-4e42-87ec-6ebe0f494fbb) - [x] scroll helper and new message indicator ([video](/attachments/958e75f2-5cd2-4594-b426-51ffa924ea12)) - [x] [attachments](/attachments/80570c1b-84d8-47d2-82f1-a6014c1be0b3) Misc - [x] the "Message" button is added to the user profile card, it takes to the latest chat with the user or starts a new one
mobile-layout.png
241 KiB
theme-settings.png
116 KiB
pleroma-dark.png
241 KiB
pleroma-light.png
262 KiB
monokai.png
227 KiB
mammal.png
250 KiB
scroll_helper.mp4
1.9 MiB
attachment.png
526 KiB
dm-timeline-update.mp4
635 KiB
dm-timeline.png
122 KiB
new-dm-user-search.mp4
1,016 KiB
delete.mp4
263 KiB
message-sequence-single-user-dm.png
128 KiB
bird.png
242 KiB
eugenijm commented 2019-12-05 21:45:34 +00:00
Author
Member
Copy link

cc @feld @hj @kaniini @lambadalambda @shpuld ready for feedback / review

cc @feld @hj @kaniini @lambadalambda @shpuld ready for feedback / review
lambadalambda commented 2019-12-06 13:41:26 +00:00
Owner
Copy link

creating a new DM does not seem to work for me, it always just switches to the last DM conversation i had instead of making a new one.

creating a new DM does not seem to work for me, it always just switches to the last DM conversation i had instead of making a new one.
eugenijm commented 2019-12-06 14:37:09 +00:00
Author
Member
Copy link

Oh, I thought that would be the desired behavior since that's how twitter DM UI works too (if the DM with the selected user already exists, it redirects to it). I will change it to always starting a new conversation then.

Oh, I thought that would be the desired behavior since that's how twitter DM UI works too (if the DM with the selected user already exists, it redirects to it). I will change it to always starting a new conversation then.
hj commented 2019-12-06 14:50:40 +00:00
Owner
Copy link

starting new blank convesation with same user sounds confusing, even if you delete old one....

starting new blank convesation with same user sounds confusing, even if you delete old one....
lambadalambda commented 2019-12-06 15:37:20 +00:00
Owner
Copy link

no, i mean, it goes to the last dm conversation i had, with a different user.

no, i mean, it goes to the last dm conversation i had, with a different user.
eugenijm commented 2019-12-06 16:26:29 +00:00
Author
Member
Copy link

Sorry, got it, sounds like a bug. This might be due to BE not being new enough. This feature relies on the recipient filter that was added to BE in 7888803ffe. So if the BE doesn't have this filter yet, the FE gets the latest conversation regardless of the passed recipients. Edit: for extra safety added sanity check to make it ignore such BE response.

Sorry, got it, sounds like a bug. This might be due to BE not being new enough. This feature relies on the recipient filter that was added to BE in `7888803ffe`. So if the BE doesn't have this filter yet, the FE gets the latest conversation regardless of the passed recipients. Edit: for extra safety added sanity check to make it ignore such BE response.
hj commented 2019-12-07 20:48:11 +00:00
Owner
Copy link

what's up with this change?

what's up with this change?
hj commented 2019-12-07 20:50:46 +00:00
Owner
Copy link

probably better to use mapState, see src/components/status/status.js:315

probably better to use mapState, see src/components/status/status.js:315
hj commented 2019-12-07 20:58:25 +00:00
Owner
Copy link

i'm not sure about this, it could be annoying on mobile because it automatically brings up keyboard reducing screen space even if you don't intend to search

i'm not sure about this, it could be annoying on mobile because it automatically brings up keyboard reducing screen space even if you don't intend to search
hj commented 2019-12-07 20:59:37 +00:00
Owner
Copy link

is it necessary to clean DMs every time? I feel like a proper GC might be better instead.

is it necessary to clean DMs every time? I feel like a proper GC might be better instead.
hj commented 2019-12-07 21:11:38 +00:00
Owner
Copy link

so is it conversationPage or conversationPageNew?

so is it conversationPage or conversationPageNew?
hj commented 2019-12-07 21:16:50 +00:00
Owner
Copy link

;;;;;;;;

;;;;;;;;
hj commented 2019-12-07 21:17:04 +00:00
Owner
Copy link

also, theme support?

also, theme support?
hj commented 2019-12-07 21:18:04 +00:00
Owner
Copy link

can we use existing component styles and not make new ones?

can we use existing component styles and not make new ones?
hj commented 2019-12-07 21:18:27 +00:00
Owner
Copy link

??

??
hj commented 2019-12-07 21:21:06 +00:00
Owner
Copy link

can you please at least commend what those slices do or intend to do?

can you please at least commend what those slices do or intend to do?
hj commented 2019-12-07 21:21:40 +00:00
Owner
Copy link

mapState for currentUser

`mapState` for `currentUser`
hj commented 2019-12-07 21:22:10 +00:00
Owner
Copy link

branch looks pointless

branch looks pointless
hj commented 2019-12-07 21:24:07 +00:00
Owner
Copy link
  1. not sure you even need the $event part, i think it's essentially same as @click="toggleMenu"
  2. maybe open menu only on right click? it's annoying to have menus open up on left click
1. not sure you even need the `$event` part, i think it's essentially same as `@click="toggleMenu"` 2. maybe open menu only on right click? it's annoying to have menus open up on left click
hj commented 2019-12-07 21:24:27 +00:00
Owner
Copy link

what's up with h4?

what's up with `h4`?
hj commented 2019-12-07 21:25:11 +00:00
Owner
Copy link

can you problably move whole browser-specific thing into a separate method?

can you problably move whole browser-specific thing into a separate method?
hj commented 2019-12-07 21:30:05 +00:00
Owner
Copy link

an image is not necessarily a photo

an image is not necessarily a photo
hj commented 2019-12-07 21:31:16 +00:00
Owner
Copy link

i'd much prefer if we used standard tools (i.e. getLocalizedDate or something) instead of trying to localize dates ourselves, it's gonna end up badly

i'd much prefer if we used standard tools (i.e. `getLocalizedDate` or something) instead of trying to localize dates ourselves, it's gonna end up badly
hj commented 2019-12-07 21:35:07 +00:00
Owner
Copy link

can you name it differently, like initializeDirectConversationsStreaming?

can you name it differently, like `initializeDirectConversationsStreaming`?
hj commented 2019-12-07 21:35:23 +00:00
Owner
Copy link

do you even need to fetch that every time? or at all?

do you even need to fetch that every time? or at all?
hj commented 2019-12-07 21:43:36 +00:00
Owner
Copy link

overall it feels like instead of adding direct conversations into pleroma-fe it adds an entire different application that supports direct conversations to pleroma-fe. It's incredibly difficult to review and will be even harder to maintain. Code Reuse is nowhere to be found.

Here's a list of what needs to be done

  1. Refactor status by extracting post content into a PostContent component and reuse it between status and direct conversation status
  2. Reuse post status form, refactor it as needed.
  3. Rename scroll-helper to make it more clear what it does.
  4. Either remove sticky-panel-header or make it a separate component or something, we want that for other timelines too in the future.
  5. Sorta repeat since 1 and 2 already imply it, but - consistent style of post form and attachments.
overall it feels like instead of adding direct conversations into pleroma-fe it adds an entire different application that supports direct conversations to pleroma-fe. It's incredibly difficult to review and will be even harder to maintain. Code Reuse is nowhere to be found. Here's a list of what **needs to be done** 1. Refactor status by extracting post content into a `PostContent` component and reuse it between status and direct conversation status 2. Reuse post status form, refactor it as needed. 3. Rename scroll-helper to make it more clear what it does. 4. Either remove sticky-panel-header or make it a separate component or something, we want that for other timelines too in the future. 5. Sorta repeat since 1 and 2 already imply it, but - consistent style of post form and attachments.
hj commented 2019-12-07 22:11:43 +00:00
Owner
Copy link

some testing:

  1. when conversation only has one message on a tall screen it gets pinned to the very bottom of it, should start at the top imo.
  2. going to conversation and back jumps the entire ui to and fro, scrollbar also blinks
  3. messages by other users look clickable (pointer cursor) but you can't interact with them
  4. Backend issue i told you dog
    image
    Backend treats public thread that contains a DM as a direct conversation, should probably only start at direct conversation. cc @lambadalambda
    FE should probably properly show that some posts are in fact public
  5. See screenshot above - custom emojis are broken
  6. odd whitespace:
    image
  7. unable to create DMs the old way:
    image
  8. timestamp overlaps the message:
    image
  9. "More" at the bottom does not have a pointer cursor, once loading starts it never stops
  10. Backend issue i see a lot of threads that are 100% public
  11. viewing an old thread brings it to the top of the list. JUST VIEWING, not posting into it.
  12. post status form looks awful on Redmond themes (see screenshot below)
  13. uploading too many attachments breaks the form:
    image
  14. Arrow in panel header doesn't use proper color (should use: panel-link):
    image
  15. notifications indicator horribly overflows:
    image
  16. uploading image shows "You: Photo", uploading video shows "You: Video", uploading anything else shows "You:"
  17. When there's only one avatar for conversation clicking on avatar (opens up conversation view) and then clicking back directs you to that user's profile instead of conversation list.
  18. Old "Direct Messages" link is gone.
some testing: 1. when conversation only has one message on a tall screen it gets pinned to the very bottom of it, should start at the top imo. 2. going to conversation and back jumps the entire ui to and fro, scrollbar also blinks 3. messages by other users look clickable (pointer cursor) but you can't interact with them 4. **Backend issue** ***i told you dog*** ![image](/attachments/160fffab-1003-420e-afd3-57f02d89f59b) Backend treats public thread that contains a DM as a direct conversation, should probably only start at direct conversation. cc @lambadalambda FE should probably properly show that some posts are in fact public 5. See screenshot above - custom emojis are broken 6. odd whitespace: ![image](/attachments/9f05e6c7-2058-4826-ba16-5f950d5d5f33) 7. unable to create DMs the old way: ![image](/attachments/b80e7480-ee4a-4f48-8dc1-7b7e6b7ce2d4) 8. timestamp overlaps the message: ![image](/attachments/c7c42dd9-37d3-4eb6-b9df-4056363055f1) 9. "More" at the bottom does not have a pointer cursor, once loading starts it never stops 10. **Backend issue** i see a lot of threads that are 100% public 11. viewing an old thread brings it to the top of the list. JUST VIEWING, not posting into it. 12. post status form looks awful on Redmond themes (see screenshot below) 13. uploading too many attachments breaks the form: ![image](/attachments/dc5eac89-7fb6-4a69-95a2-155218188de3) 14. Arrow in panel header doesn't use proper color (should use: panel-link): ![image](/attachments/7f9698f5-4cf4-446d-902f-c3c5a740c11d) 15. notifications indicator horribly overflows: ![image](/attachments/4b2fd508-8519-4ce7-a47c-f4d9e9db76f4) 16. uploading image shows "You: Photo", uploading video shows "You: Video", uploading anything else shows "You:" 17. When there's only one avatar for conversation clicking on avatar (opens up conversation view) and then clicking back directs you to that user's profile instead of conversation list. 18. Old "Direct Messages" link is gone.
image.png
2.9 KiB
image.png
14 KiB
image.png
749 B
image.png
1.9 KiB
image.png
363 KiB
image.png
36 KiB
image.png
6.7 KiB
eugenijm commented 2019-12-09 09:42:48 +00:00
Author
Member
Copy link

Hmm, it doesn't bring up keyboard for me when I test it on mobile. Looks like it was intentionally made impossible to do so so as not to annoy users

Hmm, it doesn't bring up keyboard for me when I test it on mobile. Looks like it was intentionally made impossible to do so so as not to annoy users
eugenijm commented 2019-12-09 11:23:02 +00:00
Author
Member
Copy link

it's in the link preview card

it's in the link preview card
eugenijm commented 2019-12-10 05:46:23 +00:00
Author
Member
Copy link
  1. Refactor status by extracting post content into a PostContent component and reuse it between status and direct conversation status

Will do that. What about displaying attachments in TL and direct conversation? There is a common issue with layouts that needs to be kept scrolled to the bottom: to avoid layout "jumping" when new images get downloaded, FE should either know the dimensions in advance or use fixed height. Since the dimensions are missing unless a custom BE upload filter is used, I used fixed height. Is this something we want to unify with TL too?

  1. Either remove sticky-panel-header or make it a separate component or something, we want that for other timelines too in the future.

Re: using sticky-panel-header for other timelines, does it mean removing or doing something about the space between the top bar and the sidebar / TL? Right now, there is an asymmetry when the sticky header is used: screenshot. If we keep it at the same level, then the TL content would appear on top of the background image. So we can either remove the space or replace it with something that would be on top of the TL content and the background image.

  1. Sorta repeat since 1 and 2 already imply it, but - consistent style of post form and attachments.

Re: posting form, which style should attachments have, the one that is in direct conversations (attachments are inside the form), or the one used in the main posting form (attachments are outside the form)?

Also, I had to change textarea to div with contenteditable to make it work properly with the mobile layout. With textarea, there was white space appearing at the bottom of the form in iOS Safari (screenshot). I tried different approaches to get rid of it, but switching to contenteditable with some minor global layout adjustments was the only thing that worked. Should we use div contenteditable for the main posting form too? (pros: makes possible to display emojis inside input, cons: less convenient API for autocomplete)

> 1. Refactor status by extracting post content into a `PostContent` component and reuse it between status and direct conversation status Will do that. What about displaying attachments in TL and direct conversation? There is a common issue with layouts that needs to be kept scrolled to the bottom: to avoid layout "jumping" when new images get downloaded, FE should either know the dimensions in advance or use fixed height. Since the dimensions are missing unless a custom BE upload filter is used, I used fixed height. Is this something we want to unify with TL too? > 4. Either remove sticky-panel-header or make it a separate component or something, we want that for other timelines too in the future. Re: using sticky-panel-header for other timelines, does it mean removing or doing something about the space between the top bar and the sidebar / TL? Right now, there is an asymmetry when the sticky header is used: [screenshot](/attachments/c876407e-fd0b-457d-bcf9-0ba218afe845). If we keep it at the same level, then the TL content would appear on top of the background image. So we can either remove the space or replace it with something that would be on top of the TL content and the background image. > 5. Sorta repeat since 1 and 2 already imply it, but - consistent style of post form and attachments. Re: posting form, which style should attachments have, the one that is in direct conversations ([attachments are inside the form](/attachments/ed11b98d-30b9-4384-afc5-97961cd95862)), or the one used in the main posting form ([attachments are outside the form](/attachments/d9e5847b-9af7-42b2-bab5-572ee926c599))? Also, I had to change textarea to div with contenteditable to make it work properly with the mobile layout. With textarea, there was white space appearing at the bottom of the form in iOS Safari ([screenshot](/attachments/f23a1afd-c036-4471-adac-ff5bdea2e369)). I tried different approaches to get rid of it, but switching to contenteditable with some minor global layout adjustments was the only thing that worked. Should we use div contenteditable for the main posting form too? (pros: makes possible to display emojis inside input, cons: less convenient API for autocomplete)
sticky-header.png
29 KiB
direct-conversation-posting-form.png
64 KiB
main-posting-form.png
110 KiB
space.png
271 KiB
eugenijm commented 2019-12-10 08:42:47 +00:00
Author
Member
Copy link

Doesn't look like it requires theming tbh. This is for the close icon that appears on top of the attachment:

direct-conversation-posting-form

Doesn't look like it requires theming tbh. This is for the close icon that appears on top of the attachment: ![direct-conversation-posting-form](/attachments/03069475-0757-48a5-b765-b326bb3aae11)
direct-conversation-posting-form.png
64 KiB
eugenijm commented 2019-12-10 08:56:01 +00:00
Author
Member
Copy link

Without it, if we have a globally scrollable chat layout (with sticky header and input area), the transparent background wrapper is moving along with the scroll: scroll-bg.

Without it, if we have a globally scrollable chat layout (with sticky header and input area), the transparent background wrapper is moving along with the scroll: [scroll-bg](/attachments/60b9f09a-5c73-428e-b505-178c98e8e289).
scroll-bg.mp4
1.5 MiB
eugenijm commented 2019-12-10 08:58:39 +00:00
Author
Member
Copy link

Not necessary, removing that.

Not necessary, removing that.
hj commented 2019-12-10 08:58:50 +00:00
Owner
Copy link

last time i did that it did bring up keyboard, at least on Android/Firefox. And it was VERY picky about focus, too, like even small blur event will make keyboard disappear, iirc even just switching to another field.

anyway, needs ux testing.

last time i did that it did bring up keyboard, at least on Android/Firefox. And it was VERY picky about focus, too, like even small blur event will make keyboard disappear, iirc even just switching to another field. anyway, needs ux testing.
hj commented 2019-12-10 09:01:42 +00:00
Owner
Copy link

the entirety of it?

the entirety of it?
hj commented 2019-12-10 09:03:22 +00:00
Owner
Copy link

also, if i understand, toggleMenu doesn't actually toggles any menu, it just shows "Delete" underneath a post, probably should be more like toggleActions or something? Not to mention if that's how i understood it it doesn't really matter preventing display of it on links and whatnot.

also, if i understand, `toggleMenu` doesn't actually toggles any menu, it just shows "Delete" underneath a post, probably should be more like `toggleActions` or something? Not to mention if that's how i understood it it doesn't really matter preventing display of it on links and whatnot.
hj commented 2019-12-10 09:20:19 +00:00
Owner
Copy link

Will do that. What about displaying attachments in TL and direct conversation? There is a common issue with layouts that needs to be kept scrolled to the bottom: to avoid layout "jumping" when new images get downloaded, FE should either know the dimensions in advance or use fixed height. Since the dimensions are missing unless a custom BE upload filter is used, I used fixed height. Is this something we want to unify with TL too?

Yes, the more unifying between TL and DC the better, IIRC TL attachments component is already quite fine-tuned to the extreme to handle dynamic aspects ratios while using fixed heights.

using sticky-panel-header for other timelines

yeah, the asymmetry looks ugly and we have to find a way around it (i'm thinking about css masks but not necessary, we could make timeline itself scrollable instead of page which also sorta in-line with potential multi-column setup), imo it should be removed for now and re-added separately for all panels, it's a sorta requirement for upcoming navigation overhaul https://shigusegubu.club/notice/9pC2lOupzsfX97M2dc (no issue made for it yet)

Re: posting form, which style should attachments have, the one that is in direct conversations

I would really appreciate if you just used PostStatusForm with minimal changes in it right now. There's no need to follow some sort of design mockup right now, it's more important to make it consistent across the application.

Also, I had to change textarea to div with contenteditable

again, just use textarea for now.

>Will do that. What about displaying attachments in TL and direct conversation? There is a common issue with layouts that needs to be kept scrolled to the bottom: to avoid layout "jumping" when new images get downloaded, FE should either know the dimensions in advance or use fixed height. Since the dimensions are missing unless a custom BE upload filter is used, I used fixed height. Is this something we want to unify with TL too? Yes, the more unifying between TL and DC the better, IIRC TL attachments component is already quite fine-tuned to the extreme to handle dynamic aspects ratios while using fixed heights. >using sticky-panel-header for other timelines yeah, the asymmetry looks ugly and we have to find a way around it (i'm thinking about css masks but not necessary, we could make timeline itself scrollable instead of page which also sorta in-line with potential multi-column setup), imo it should be removed for now and re-added separately for all panels, it's a sorta requirement for upcoming navigation overhaul https://shigusegubu.club/notice/9pC2lOupzsfX97M2dc (no issue made for it yet) >Re: posting form, which style should attachments have, the one that is in direct conversations I would really appreciate if you just used PostStatusForm with minimal changes in it right now. There's no need to follow some sort of design mockup right now, it's more important to make it consistent across the application. >Also, I had to change textarea to div with contenteditable again, just use textarea for now.
hj commented 2019-12-10 09:57:10 +00:00
Owner
Copy link

looks like it's mostly because you position the entire thing absolutely or something like that

looks like it's mostly because you position the entire thing absolutely or something like that
eugenijm commented 2019-12-11 11:21:26 +00:00
Author
Member
Copy link

On a second thought, maybe it's unlikely that BE would need to provide a different URL, so let's just use location.origin.replace(/(http)(s)?:\/\//, 'ws$2://') to get the streaming URL then?

On a second thought, maybe it's unlikely that BE would need to provide a different URL, so let's just use `location.origin.replace(/(http)(s)?:\/\//, 'ws$2://')` to get the streaming URL then?
eugenijm commented 2019-12-11 11:39:16 +00:00
Author
Member
Copy link

re: right click, I'd think most users don't expect it to work on web and I think it doesn't work on most tablets? The idea to open it on left click was basically taken from the new twitter DM UI thinking that this is what most users will learn to expect over time

re: right click, I'd think most users don't expect it to work on web and I think it doesn't work on most tablets? The idea to open it on left click was basically taken from the new twitter DM UI thinking that this is what most users will learn to expect over time
eugenijm commented 2019-12-11 11:44:17 +00:00
Author
Member
Copy link

the early return part is for the scenario when you click on link / link preview and it shows the "Delete" at the same time

the early return part is for the scenario when you click on link / link preview and it shows the "Delete" at the same time
hj commented 2019-12-11 11:47:47 +00:00
Owner
Copy link

i guess if all it does is show actions then left click is fine, right click is also used on web (see: youtube) for mobiles it's long tap usually. Problem is that with name toggleMenu you'd think it toggles (context) menu, but it does not.

As long as actions doesn't make whole thing jump i don't see the big need for early returns

i guess if all it does is show actions then left click is fine, right click is also used on web (see: youtube) for mobiles it's long tap usually. Problem is that with name `toggleMenu` you'd think it toggles (context) menu, but it does not. As long as actions doesn't make whole thing jump i don't see the big need for early returns
hj commented 2019-12-11 11:50:47 +00:00
Owner
Copy link

just use existing instance url info like here #2279/diffs

on the other hand probably worth waiting for that MR to be merged and use ParsedWS from there?

just use existing instance url info like here https://git.pleroma.social/pleroma/pleroma-fe/pulls/2279/diffs#0ced30d71bd77d681d55787b4adcca61fbc46b7c_229_24 on the other hand probably worth waiting for that MR to be merged and use `ParsedWS` from there?
succfemboi commented 2019-12-12 13:14:58 +00:00
Member
Copy link

Fedilab also shows the same back end issue in production

Fedilab also shows the same back end issue in production
eugenijm commented 2019-12-17 20:50:16 +00:00
Author
Member
Copy link

Sure, will use ParsedWS once it's merged

Sure, will use `ParsedWS` once it's merged
eugenijm commented 2019-12-17 20:54:13 +00:00
Author
Member
Copy link

Yes, the more unifying between TL and DC the better, IIRC TL attachments component is already quite fine-tuned to the extreme to handle dynamic aspects ratios while using fixed heights.

All done. Would it be okay to make a separate MR for the extracted StatusContent? I think having it merged first would make it easier to proceed with this MR and the review.

we could make timeline itself scrollable instead of page which also sorta in-line with potential multi-column setup

Changed it to be scrollable inside the TL screenshot so that there is no asymmetry and it's easier to unify with other TLs

I would really appreciate if you just used PostStatusForm with minimal changes in it right now. There's no need to follow some sort of design mockup right now, it's more important to make it consistent across the application.

Alright, switched to PostStatusForm

> Yes, the more unifying between TL and DC the better, IIRC TL attachments component is already quite fine-tuned to the extreme to handle dynamic aspects ratios while using fixed heights. All done. Would it be okay to make a separate MR for the extracted `StatusContent`? I think having it merged first would make it easier to proceed with this MR and the review. > we could make timeline itself scrollable instead of page which also sorta in-line with potential multi-column setup Changed it to be scrollable inside the TL [screenshot](/attachments/a4d8d3b3-dd0a-4f7a-b1ad-db3e0930f237) so that there is no asymmetry and it's easier to unify with other TLs > I would really appreciate if you just used PostStatusForm with minimal changes in it right now. There's no need to follow some sort of design mockup right now, it's more important to make it consistent across the application. Alright, switched to `PostStatusForm`
chat-layout2.png
223 KiB
eugenijm commented 2019-12-17 21:20:52 +00:00
Author
Member
Copy link
  1. when conversation only has one message on a tall screen it gets pinned to the very bottom of it, should start at the top imo.

Changed it to start at the top

  1. going to conversation and back jumps the entire ui to and fro, scrollbar also blinks

Should be better now: fixed scroll blinking and DC TL being reset

  1. FE should probably properly show that some posts are in fact public

Added a tiny visibility indicator: screenshot

7 unable to create DMs the old way:
18 Old "Direct Messages" link is gone.

Returned the old DM links

  1. viewing an old thread brings it to the top of the list. JUST VIEWING, not posting into it.

Most likely this was because it was unread and reading it refreshed the updated_at timestamp on BE, so basically a BE issue (made a MR with the fix).

  1. post status form looks awful on Redmond themes (see screenshot below)

Updated the Redmond themes: screenshot

  1. uploading too many attachments breaks the form:

Switched to PostStatusForm as you suggested so that we can unify how multiple uploads are handled

3, 5, 6, 14, 16, 15, 17: fixed

> 1. when conversation only has one message on a tall screen it gets pinned to the very bottom of it, should start at the top imo. Changed it to start at the top > 2. going to conversation and back jumps the entire ui to and fro, scrollbar also blinks Should be better now: fixed scroll blinking and DC TL being reset > 4. FE should probably properly show that some posts are in fact public Added a tiny visibility indicator: [screenshot](/attachments/fe41ba67-3a1a-4136-a792-50985073a72f) > 7 unable to create DMs the old way: > 18 Old "Direct Messages" link is gone. Returned the old DM links > 11. viewing an old thread brings it to the top of the list. JUST VIEWING, not posting into it. Most likely this was because it was unread and reading it refreshed the `updated_at` timestamp on BE, so basically a BE issue (made a MR with the fix). > 12. post status form looks awful on Redmond themes (see screenshot below) Updated the Redmond themes: [screenshot](/attachments/a466caff-4ea0-432b-bf0b-12d517e6ba7e) > 13. uploading too many attachments breaks the form: Switched to `PostStatusForm` as you suggested so that we can unify how multiple uploads are handled 3, 5, 6, 14, 16, 15, 17: fixed
scope-visibility.png
5.6 KiB
redmond.png
22 KiB
eugenijm commented 2019-12-17 21:24:52 +00:00
Author
Member
Copy link

resolved since the TL is now scrollable screenshot

resolved since the TL is now scrollable [screenshot](/attachments/7fd17ecd-a02c-4042-9112-7a6903207c06)
chat-layout2.png
223 KiB
eugenijm commented 2019-12-22 18:49:19 +00:00
Author
Member
Copy link

made it more precise with closest('.link-preview-card, .attachments')

made it more precise with `closest('.link-preview-card, .attachments')`
hj commented 2019-12-23 05:52:18 +00:00
Owner
Copy link

Some more testing of updated version:

  1. timestamp still overlaps with text
    image
  2. Typing a message and sending it doesn't give you any feedback - message doesn't appear in the log immediately but only after 2-3 seconds delay
  3. File upload icon get hover effect when empty space next to it is hovered, but it cannot be clicked there
    image
  4. Emoji icon unjustly missing
  5. "Direct Conversations", right next to "Direct Messages" bearing same icon is quite confusing, I'd propose using some sort of "chat bubble" icon and calling it just "Conversations", more on that later.
  6. Direct Messages page seems to be broken, XHR it sends has exclude_visibilities[]=direct which makes whole page broken.
  7. Some people might have too long nicknames and/or nicknames with custom emojis in, this makes placeholder text in input a bit... unrefined. Probably should use @handle instead
    image
  8. User handle included in first message but not in following. Probably should be outside the message, not inside.
    image
  9. User avatar should probably animate when message(s?) are hovered, not just avatar itself, like original posts are.
  10. same as "Images are not necessarily Photos" - files are not necessarily Documents, so i suggest changing Document to File
  11. Message boxes should probably allow customizing roundness
  12. Conversations list has A LOT of blue to it - mentions, at-handles, links from message preview, "You:" etc. Most of them SEEM clickable (it gets a slight border when clicked) but doesn't do much different from opening a conversation, so:
    1. Use faint text for message preview?
    2. Disable browser's "link clicked" style
    3. If conversation is 1-on-1 there's probably no need to show handle: message unless it's "You:"
    4. It's not placeholder text so we can easily use nicknames there instead of handles - even render custom emojis there.
    5. Overall it's probably better to use similar display to post, i.e.
      Nickname user@handle
      user handle should probably use linkFaint tho.
  13. Delete link positioned very awkwardly
    image
  14. More of a small request than a bug - clicking timestamp should open up that post in traditional view, or add another link next to "Delete" - so that it would be easier to debug what post looks like in traditional view.
  15. Can't add polls either, not sure about polls display, but it does have use for DMs, like asking 5-10 people if they want to go for a pizza, burgers or straight to beer.

And now the big one.

The whole thing is a UX mess. When we roll out this feature users will be greeted with 100+ unread "direct conversations" with 80% of them not even being "direct" at all, just regular threads or even messages incorrectly treated as DMs in the past when they were just FO. And to make matters worse - one could reply to a public post with a DM and it would mark entire thread as a DC, and no amount of BE or FE tweak will fix it - it's part of the protocol and our legacy. The only real solution that I see to that is to make direct messages exist on a separate plane - name them APType=DirectMessages2 internally or whatever - make it incompatible with traditional posts, give them extra features (like ability to delete thread/other person's messages), impose bigger restrictions (can't reply to a public post with a DM2, can't reply to a DM2 with a public post, can't favorite a DM2).

As of now, i really don't know how to fix it otherwise or what to do with whatever done. Initially I thought it's probably gonna be good, but testing it again gives me nothing but a headache and flashbacks from March when I told @feld @lambadalambda and others that this thing is a bad idea.

Direct conversations should probably be called just "Conversations" and there should be some sort of onboarding explaining why the fuck the whole thing is a mess, what purpose it serves and how to use it. As it is stands right now, I would only allow it as an experimental opt-in feature.

It reminds me of MacOS in a way - it's pretty, yeah, but limiting and clunky to the point where I'd rather use something else.

Some more testing of updated version: 1. timestamp still overlaps with text ![image](/attachments/efe55605-86df-4230-ab43-c195d958e2e2) 2. Typing a message and sending it doesn't give you any feedback - message doesn't appear in the log immediately but only after 2-3 **seconds** delay 3. File upload icon get hover effect when empty space next to it is hovered, but it cannot be clicked there ![image](/attachments/d78b376e-928a-4105-a475-10e2b2fb220e) 4. Emoji icon unjustly missing 5. "Direct Conversations", right next to "Direct Messages" bearing same icon is quite confusing, I'd propose using some sort of "chat bubble" icon and calling it just "Conversations", more on that later. 6. Direct Messages page seems to be broken, XHR it sends has `exclude_visibilities[]=direct` which makes whole page broken. 7. Some people might have too long nicknames and/or nicknames with custom emojis in, this makes placeholder text in input a bit... unrefined. Probably should use @handle instead ![image](/attachments/3c1541e0-b7b4-4561-9591-7f348087ed99) 8. User handle included in first message but not in following. Probably should be outside the message, not inside. ![image](/attachments/bc3cce25-0df8-476f-b68a-0cfdf4616329) 9. User avatar should probably animate when message(s?) are hovered, not just avatar itself, like original posts are. 10. same as "Images are not necessarily Photos" - files are not necessarily Documents, so i suggest changing `Document` to `File` 11. Message boxes should probably allow customizing roundness 12. Conversations list has A LOT of blue to it - mentions, at-handles, links from message preview, "You:" etc. Most of them SEEM clickable (it gets a slight border when clicked) but doesn't do much different from opening a conversation, so: 1. Use faint text for message preview? 2. Disable browser's "link clicked" style 3. If conversation is 1-on-1 there's probably no need to show `handle: message` unless it's "You:" 4. It's not placeholder text so we can easily use nicknames there instead of handles - even render custom emojis there. 5. Overall it's probably better to use similar display to post, i.e. **Nickname** user@handle user handle should probably use linkFaint tho. 13. Delete link positioned very awkwardly ![image](/attachments/9c6c1ca5-61bf-45f1-93dd-552ef1e894e8) 14. More of a small request than a bug - clicking timestamp should open up that post in traditional view, or add another link next to "Delete" - so that it would be easier to debug what post looks like in traditional view. 15. Can't add polls either, not sure about polls display, but it does have use for DMs, like asking 5-10 people if they want to go for a pizza, burgers or straight to beer. And now the big one. The whole thing is a UX mess. When we roll out this feature users will be greeted with **100+ unread** "direct conversations" with 80% of them not even being "direct" at all, just regular threads or even messages incorrectly treated as DMs in the past when they were just FO. And to make matters worse - one could reply to a public post with a DM and it would mark entire thread as a DC, and no amount of BE or FE tweak will fix it - it's part of the protocol and our legacy. The only real solution that I see to that is to make direct messages exist on a separate plane - name them APType=DirectMessages2 internally or whatever - make it incompatible with traditional posts, give them extra features (like ability to delete thread/other person's messages), impose bigger restrictions (can't reply to a public post with a DM2, can't reply to a DM2 with a public post, can't favorite a DM2). As of now, i really don't know how to fix it otherwise or what to do with whatever done. Initially I thought it's probably gonna be good, but testing it again gives me nothing but a headache and flashbacks from March when I told @feld @lambadalambda and others that this thing is a bad idea. Direct conversations should probably be called just "Conversations" and there should be some sort of onboarding explaining why the fuck the whole thing is a mess, what purpose it serves and how to use it. As it is stands right now, I would only allow it as an experimental opt-in feature. It reminds me of MacOS in a way - it's pretty, yeah, but limiting and clunky to the point where I'd rather use something else.
image.png
1.2 KiB
image.png
3.5 KiB
image.png
5.3 KiB
image.png
9.9 KiB
image.png
2.4 KiB
hj commented 2019-12-23 06:04:14 +00:00
Owner
Copy link
  1. ✔️
  2. ✔️
  3. ✔️
  4. ⚠️ DMs should also have same indicator
  5. ✔️
  6. ✔️
  7. ✔️
  8. ✖️ not fixed
  9. ✖️ not fixed
  10. ⚠️ already mentioned
  11. ⚠️ FE doesn't update order on itself (and also doesn't pull new DMs seemingly when in list?) but order is broken when F5 on page. Is there a BE MR and is it merged?
  12. ⚠️ doesn't reset post form properly if you delete all attachments
  13. ⚠️ see above
  14. ✔️
  15. ✔️
  16. ⚠️ mentioned that files are not necessarily documents
  17. ⚠️ should also remove browser highlight when clicking avatar
  18. ✔️
1. :heavy_check_mark: 2. :heavy_check_mark: 3. :heavy_check_mark: 4. :warning: DMs should also have same indicator 5. :heavy_check_mark: 6. :heavy_check_mark: 7. :heavy_check_mark: 8. :heavy_multiplication_x: not fixed 9. :heavy_multiplication_x: not fixed 10. :warning: already mentioned 11. :warning: FE doesn't update order on itself (and also doesn't pull new DMs seemingly when in list?) but order is broken when F5 on page. Is there a BE MR and is it merged? 12. :warning: doesn't reset post form properly if you delete all attachments 13. :warning: see above 14. :heavy_check_mark: 15. :heavy_check_mark: 16. :warning: mentioned that files are not necessarily documents 17. :warning: should also remove browser highlight when clicking avatar 18. :heavy_check_mark:
hj commented 2019-12-23 06:17:46 +00:00
Owner
Copy link

another bug:
image
this causes horizontal scroll to appear

another bug: ![image](/attachments/b54031c7-c1de-463d-8646-9735a0ce9d6c) this causes horizontal scroll to appear
image.png
34 KiB
hj commented 2019-12-23 06:19:33 +00:00
Owner
Copy link

what's up with 'avatar-compact': false?

what's up with `'avatar-compact': false`?
hj commented 2019-12-23 06:20:14 +00:00
Owner
Copy link

what's up with this global scope pollution?

what's up with this global scope pollution?
hj commented 2019-12-23 06:23:00 +00:00
Owner
Copy link

??

??
hj commented 2019-12-23 06:24:44 +00:00
Owner
Copy link

what's up with this fallback?

what's up with this fallback?
hj commented 2019-12-23 06:26:09 +00:00
Owner
Copy link

what's up with this indentation?

what's up with this indentation?
hj commented 2019-12-23 06:31:31 +00:00
Owner
Copy link

https://kazupon.github.io/vue-i18n/guide/pluralization.html

https://kazupon.github.io/vue-i18n/guide/pluralization.html
hj commented 2019-12-23 06:31:51 +00:00
Owner
Copy link

should be an error maybe?

should be an error maybe?
hj commented 2019-12-23 06:32:41 +00:00
Owner
Copy link

directConversations.forEach?

`directConversations.forEach`?
hj commented 2019-12-23 06:33:09 +00:00
Owner
Copy link

const?

`const`?
hj commented 2019-12-23 06:34:05 +00:00
Owner
Copy link

why?

why?
hj commented 2019-12-23 06:39:23 +00:00
Owner
Copy link

instead of Alt it should be Incoming and Outgoing

also both of them should have their own Text and Link definitions

I.e. you may want make iOS-style white-on-blue/black-on-grey messages but shared "text" for both incoming and outgoing won't let you.

instead of `Alt` it should be `Incoming` and `Outgoing` also both of them should have their own Text and Link definitions I.e. you may want make iOS-style white-on-blue/black-on-grey messages but shared "text" for both incoming and outgoing won't let you.
hj commented 2019-12-23 06:39:44 +00:00
Owner
Copy link

should probably use Border instead of Fg, it's confusing otherwise.

should probably use `Border` instead of `Fg`, it's confusing otherwise.
hj commented 2019-12-23 06:41:32 +00:00
Owner
Copy link

doesn't seem to be used anymore

doesn't seem to be used anymore
hj commented 2019-12-23 06:42:49 +00:00
Owner
Copy link

image convo pane doesn't respect transparency

![image](/attachments/3237f742-8e1f-487d-98b3-471dd8280103) convo pane doesn't respect transparency
image.png
286 KiB
hj commented 2019-12-23 06:48:08 +00:00
Owner
Copy link

i get a feeling this might pollute defaultState, should probably use clone - { ...defaultState }

i get a feeling this might pollute `defaultState`, should probably use clone - `{ ...defaultState }`
hj commented 2019-12-23 06:48:46 +00:00
Owner
Copy link

not critical but makes me nervous

not critical but makes me nervous
hj commented 2019-12-23 06:51:03 +00:00
Owner
Copy link

I don't see stuff in status_content.js removed from status.js

I don't see stuff in `status_content.js` removed from `status.js`
hj commented 2019-12-23 06:51:43 +00:00
Owner
Copy link

weird

weird
hj commented 2019-12-23 06:57:15 +00:00
Owner
Copy link

i feel like this is misleading and confusing since two post forms are so identical you sorta expect them to behave the same, but this pulls the rug under user by submitting on enter.

ideally it should be customizable globally and there should be a hint text "Enter will send post" or "Shift-Enter will post"

i feel like this is misleading and confusing since two post forms are so *identical* you sorta expect them to behave the same, but this pulls the rug under user by submitting on enter. ideally it should be customizable globally and there should be a hint text "Enter will send post" or "Shift-Enter will post"
hj commented 2019-12-23 06:57:30 +00:00
Owner
Copy link

maybe just placeholder?

maybe just `placeholder`?
hj commented 2019-12-23 07:01:09 +00:00
Owner
Copy link

instead of hiding notice, Direct Conversation should have its own notice that will show whom message will be sent to.

Ideally it should also have a control to manually specify to field and display whom post will be directed to.

instead of hiding notice, Direct Conversation should have its own notice that will show whom message will be sent to. Ideally it should also have a control to manually specify `to` field and display whom post will be directed to.
hj commented 2019-12-23 07:01:38 +00:00
Owner
Copy link

subject and noSubject sounds weird, probably disableSubject?

`subject` and `noSubject` sounds weird, probably `disableSubject`?
hj commented 2019-12-23 07:01:54 +00:00
Owner
Copy link

same applies to other noSmth

same applies to other `noSmth`
hj commented 2019-12-23 07:02:13 +00:00
Owner
Copy link

replyToDirectConversationId

`replyToDirectConversationId`
hj commented 2019-12-23 07:02:25 +00:00
Owner
Copy link

to be consistent with replyTo

to be consistent with `replyTo`
hj commented 2019-12-23 07:02:55 +00:00
Owner
Copy link

as i mentioned before - why tho? really, really, WHY

as i mentioned before - why tho? really, really, WHY
hj commented 2019-12-23 07:03:06 +00:00
Owner
Copy link

again, it's pretty useful for DMs as well.

again, it's pretty useful for DMs as well.
hj commented 2019-12-23 07:04:11 +00:00
Owner
Copy link

????

????
hj commented 2019-12-23 07:05:52 +00:00
Owner
Copy link

this is a bit overwhelming tbh

this is ***a bit*** overwhelming tbh
eugenijm commented 2019-12-23 15:53:31 +00:00
Author
Member
Copy link

the idea was to separate DMs from other posts visually, but since we're returning the old DM links there is no need for this anymore, so removing it

the idea was to separate DMs from other posts visually, but since we're returning the old DM links there is no need for this anymore, so removing it
eugenijm commented 2019-12-23 17:10:20 +00:00
Author
Member
Copy link

instead of hiding notice, Direct Conversation should have its own notice that will show whom message will be sent to.

For a 1-1 DC, the recipient is in the placeholder, for multi-user DC, they're in the header / member list: screenshot. We can also add a greeting message, e.g., "This is the beginning of your conversation with ..." when the user starts a new DC. Edit: added a notice for multi-user DMs (probably makes sense to remember it being dismissed separately since it's a new feature, will do that next)

> instead of hiding notice, Direct Conversation should have its own notice that will show whom message will be sent to. For a 1-1 DC, the recipient is in the placeholder, for multi-user DC, they're in the header / member list: [screenshot](/attachments/62bf1e0c-a11e-4001-8954-b9a4d69c60af). We can also add a greeting message, e.g., "This is the beginning of your conversation with ..." when the user starts a new DC. Edit: added a notice for multi-user DMs (probably makes sense to remember it being dismissed separately since it's a new feature, will do that next)
member-list.png
17 KiB
eugenijm commented 2019-12-25 07:45:16 +00:00
Author
Member
Copy link

changed it to IncomingBorder / OutgoingBorder

changed it to `IncomingBorder` / `OutgoingBorder`
succfemboi commented 2019-12-25 09:05:32 +00:00
Member
Copy link

Is anything else missing for this to be merged?

Is anything else missing for this to be merged?
hj commented 2019-12-25 09:15:53 +00:00
Owner
Copy link

yes, it needs more testing, more review, opt-in (unless implemented) and overall discussion about the whole mess of conversations in pleroma/pleroma-meta#21

yes, it needs more testing, more review, opt-in (unless implemented) and overall discussion about the whole mess of conversations in https://git.pleroma.social/pleroma/pleroma-meta/issues/21
eugenijm commented 2019-12-25 09:51:27 +00:00
Author
Member
Copy link

added transparency support

added transparency support
hj commented 2019-12-26 05:32:27 +00:00
Owner
Copy link

Noticed a bug that repeats of posts with NSFW content does not mark attachments as NSFW.

Noticed a bug that repeats of posts with NSFW content does not mark attachments as NSFW.
eugenijm commented 2019-12-28 06:42:38 +00:00
Author
Member
Copy link

removing that for extra safety

removing that for extra safety
eugenijm commented 2019-12-29 05:39:09 +00:00
Author
Member
Copy link

should be fixed now, tested in chrome/safari/firefox

should be fixed now, tested in chrome/safari/firefox
eugenijm commented 2020-01-05 14:33:51 +00:00
Author
Member
Copy link

removing this change for now, customization would be nice, yeah, but probably in the next iteration then

removing this change for now, customization would be nice, yeah, but probably in the next iteration then
eugenijm commented 2020-01-05 14:34:26 +00:00
Author
Member
Copy link

reverted submitDisabled changes

reverted `submitDisabled` changes
eugenijm commented 2020-01-05 14:35:43 +00:00
Author
Member
Copy link

plays strangely with the chat layout for some reason when you click on the picker, not sure why but will try to figure out

plays strangely with the chat layout for some reason when you click on the picker, not sure why but will try to figure out
eugenijm commented 2020-01-15 14:57:40 +00:00
Author
Member
Copy link

8 and 9, should be fixed now (tested on safari, chrome, firefox). Re: loading issue, similarly to bookmarks, I added the http-link-header package to use nextId from the link header so that FE doesn't depend on internal BE ordering.

8 and 9, should be fixed now (tested on safari, chrome, firefox). Re: loading issue, similarly to bookmarks, I added the `http-link-header` package to use `nextId` from the link header so that FE doesn't depend on internal BE ordering.
eugenijm commented 2020-01-16 16:54:24 +00:00
Author
Member
Copy link

To prevent it from jumping, switched to a popover screenshot

To prevent it from jumping, switched to a popover [screenshot](/attachments/14c1cef9-324e-4132-8306-f5fe0f29333a)
Screen_Shot_2020-01-16_at_7.54.33_PM.png
22 KiB
eugenijm commented 2020-01-16 17:20:42 +00:00
Author
Member
Copy link

1, 2, 3, 4, 5, 6, 7, 10, 11, 12, 13, 14, 15: fixed

8 User handle included in first message but not in following. Probably should be outside the message, not inside.

That was to avoid repeating the handle when it's the message from the same user. Maybe we could arrange message corners to make it more clear: screenshot?

9 User avatar should probably animate when message(s?) are hovered, not just avatar itself, like original posts are.

Hmm, not sure I understand this one. In original posts, avatars don't animate for me unless they are gifs and when they are gifs, they animate all the time regardless of hovering. Could you elaborate on this?

Also, added the experimental opt-in (Settings -> Experimental Features -> Direct Conversations).

1, 2, 3, 4, 5, 6, 7, 10, 11, 12, 13, 14, 15: fixed > 8 User handle included in first message but not in following. Probably should be outside the message, not inside. That was to avoid repeating the handle when it's the message from the same user. Maybe we could arrange message corners to make it more clear: [screenshot](/attachments/20c73261-eb6c-4bf4-a3aa-85932888af3c)? > 9 User avatar should probably animate when message(s?) are hovered, not just avatar itself, like original posts are. Hmm, not sure I understand this one. In original posts, avatars don't animate for me unless they are gifs and when they are gifs, they animate all the time regardless of hovering. Could you elaborate on this? Also, added the experimental opt-in (Settings -> Experimental Features -> Direct Conversations).
message-cloud.png
6.1 KiB
eugenijm commented 2020-01-16 22:02:22 +00:00
Author
Member
Copy link

11: It's been merged now pleroma/pleroma#5458.

4, 12, 13, 16, 17: fixed

11: It's been merged now https://git.pleroma.social/pleroma/pleroma/pulls/5458. 4, 12, 13, 16, 17: fixed
hj commented 2020-01-17 00:15:18 +00:00
Owner
Copy link

Could you elaborate on this?

Check out this option: image

use StillImage for avatars.

>Could you elaborate on this? Check out this option: ![image](/attachments/110e5fe2-2a55-42a1-92d0-1395c9541860) use `StillImage` for avatars.
image.png
13 KiB
hj commented 2020-01-17 00:17:26 +00:00
Owner
Copy link

I'm saying move nickname out of the bubble:

image

I'm saying move nickname out of the bubble: ![image](/attachments/8c8e6c1e-4717-48c4-840b-926c39612f8d)
image.png
9.7 KiB
eugenijm commented 2020-01-17 21:05:13 +00:00
Author
Member
Copy link

All done

All done
feld commented 2020-01-17 21:57:08 +00:00
Owner
Copy link

I need to test this branch to see how it differs from what I have been running, but the New DM functionality is terrible if the account you want to start a new message to cannot be reliably found with our search. It requires the user remember they can (hopefully) navigate to the user's profile (if they have a link or post in the timeline to help! because again, search is bad) and then hit the DM button.

So... yeah. Side effect of our search being barely usable right now for finding accounts.

I need to test this branch to see how it differs from what I have been running, but the New DM functionality is terrible if the account you want to start a new message to cannot be reliably found with our search. It requires the user remember they can (hopefully) navigate to the user's profile (if they have a link or post in the timeline to help! because again, search is bad) and then hit the DM button. So... yeah. Side effect of our search being barely usable right now for finding accounts.
eugenijm commented 2020-02-05 21:01:15 +00:00
Author
Member
Copy link

This is fixed now

This is fixed now
hj commented 2020-04-21 17:31:22 +00:00
Owner
Copy link

should be hidden when direct conversations are turned off

should be hidden when direct conversations are turned off
hj commented 2020-04-21 17:31:35 +00:00
Owner
Copy link

also should probably in the menu but not as important rn

also should probably in the menu but not as important rn
hj commented 2020-04-21 17:55:14 +00:00
Owner
Copy link

@eugenijm @shpuld @lambadalambda I think this just needs one small fix to hide dm button in user profile (it's always shown), but otherwise should be ok to merge this and fix any issues that arise later.

Direct conversations are off by default and hidden rather deep in settings. This should allow to test and improve the API/backend side of things easier, on top of that there's some nice refactoring in there that separates status from its contents which would help refactoring notifications to make them lighter.

@eugenijm @shpuld @lambadalambda I think this just needs one small fix to hide dm button in user profile (it's always shown), but otherwise should be ok to merge this and fix any issues that arise later. Direct conversations are off by default and hidden rather deep in settings. This should allow to test and improve the API/backend side of things easier, on top of that there's some nice refactoring in there that separates status from its contents which would help refactoring notifications to make them lighter.
lanodan commented 2020-04-23 15:14:34 +00:00
Owner
Copy link

Great, will merge it in my branch and test it a bit.

Update: Works nicely.

Great, will merge it in my branch and test it a bit. Update: Works nicely.
rinpatch commented 2020-04-24 17:59:00 +00:00
Member
Copy link

This overflows when markdown code blocks are used image

This overflows when markdown code blocks are used ![image](/attachments/bdf2ea37-e813-4b8d-abfe-1b52179d7692)
image.png
200 KiB
hj commented 2020-04-24 18:24:51 +00:00
Owner
Copy link

i'd prefer to fix problems not related to conversations themselves first, so that we could merge it to take advantage of refactoring done in this MR, and then fix conversation-related issues separately, keeping conversations off by default as an experimental feature until feature stabilizes and becomes more usable as well

i'd prefer to fix problems not related to conversations themselves first, so that we could merge it to take advantage of refactoring done in this MR, and then fix conversation-related issues separately, keeping conversations off by default as an experimental feature until feature stabilizes and becomes more usable as well
shpuld commented 2020-04-27 08:47:51 +00:00
Owner
Copy link

Some problems I found when testing:

  • plain enter doesn't send, makes sense for regular statuses but not so much for dms
  • the chat bubbles should probably respect panel border radius, they look very wrong in default themes now.
  • toggling the "add poll" ui messes up the layout
  • going back and forth between conversations list and a conversation looks glitchy, conversation should probably always take full height to help with this
  • suggestions go off screen when they expand down
Some problems I found when testing: - plain enter doesn't send, makes sense for regular statuses but not so much for dms - the chat bubbles should probably respect panel border radius, they look very wrong in default themes now. - toggling the "add poll" ui messes up the layout - going back and forth between conversations list and a conversation looks glitchy, conversation should probably always take full height to help with this - suggestions go off screen when they expand down
shpuld commented 2020-04-27 09:21:51 +00:00
Owner
Copy link

I'm going to ease this MR a bit by separating the status -> statuscontent refactor into a smaller MR

I'm going to ease this MR a bit by separating the status -> statuscontent refactor into a smaller MR
hj commented 2020-04-27 09:24:19 +00:00
Owner
Copy link

i'm not sure about send-on-enter, it seems a bit confusing to have different approaches to same thing. On top of that, some people voiced that they want send-on-enter in main input too.

Don't really want to say it, but it's probably better be an option to control input forms separately? Not sure if that or actual full-blown hotkey system, which was also asked about I believe but hasn't really been a necessity yet.

i'm not sure about send-on-enter, it seems a bit confusing to have different approaches to same thing. On top of that, some people voiced that they want send-on-enter in main input too. Don't really want to say it, but it's probably better be an option to control input forms separately? Not sure if that or actual full-blown hotkey system, which was also asked about I believe but hasn't really been a necessity yet.
eugenijm commented 2020-05-07 13:47:23 +00:00
Author
Member
Copy link

Updated the MR to make it use the new Chat API instead of Conversations, renamed Direct Conversations to Chats

Updated the MR to make it use [the new Chat API](https://git.pleroma.social/pleroma/pleroma/pulls/5807) instead of Conversations, renamed Direct Conversations to Chats - [desktop](/attachments/f567507e-d4c6-46e6-905f-55cb6b4a35d3) - [mobile](/attachments/9ada8503-8ab7-47c0-9d3e-d62d42c5080e)
new-chat.png
152 KiB
new-chat-mobile.png
28 KiB
lambadalambda commented 2020-05-07 15:55:38 +00:00
Owner
Copy link

I installed both on the test instances, overall it works, but there's some weirdness. Sending a message blocks the input field and I can't send another one without refreshing

I installed both on the test instances, overall it works, but there's some weirdness. Sending a message blocks the input field and I can't send another one without refreshing
lambadalambda commented 2020-05-07 16:03:12 +00:00
Owner
Copy link

It also only inserts the message after the notification arrives, i think. I guess it somehow doesn't correctly use the 200 OK from the message post.

It also only inserts the message after the notification arrives, i think. I guess it somehow doesn't correctly use the 200 OK from the message post.
lambadalambda commented 2020-05-07 16:08:54 +00:00
Owner
Copy link

there also seems to be some ordering issue

there also seems to be some ordering issue
lambadalambda commented 2020-05-07 16:18:35 +00:00
Owner
Copy link

Screenshot_2020-05-07__1__dontbulling_me_3_

![Screenshot_2020-05-07__1__dontbulling_me_3_](/attachments/30e7a2ad-c4c8-4b81-b680-7bc92b9fabbd)
Screenshot_2020-05-07__1__dontbulling_me_3_.png
141 KiB
eugenijm commented 2020-05-07 16:49:34 +00:00
Author
Member
Copy link

Ah, fixed the message sending, inserting and, the ordering issue .

Ah, fixed the message sending, inserting and, the ordering issue .
lambadalambda commented 2020-05-07 16:58:37 +00:00
Owner
Copy link

One more thing, I find it quite hard to see which part of the conversation is supposed to be me and which is the other one. I think the other side of the chat should have a little avatar attached

One more thing, I find it quite hard to see which part of the conversation is supposed to be me and which is the other one. I think the other side of the chat should have a little avatar attached
shpuld commented 2020-05-07 17:31:52 +00:00
Owner
Copy link

while it's super common to not have any indication, right side for your posts is pretty standard in most chat applications I've used, it would be necessary to add names or avatars later for group chats so might as well do it now already for consistent UI

while it's super common to not have any indication, right side for your posts is pretty standard in most chat applications I've used, it would be necessary to add names or avatars later for group chats so might as well do it now already for consistent UI
lambadalambda commented 2020-05-07 18:23:12 +00:00
Owner
Copy link

Can confirm that the issues i reported are fixed

Can confirm that the issues i reported are fixed
lambadalambda commented 2020-05-07 18:23:56 +00:00
Owner
Copy link

all the ones i use (conversation, riot, line) have some kind of indication, usually one small avatar image above the first of a chain of messages.

all the ones i use (conversation, riot, line) have some kind of indication, usually one small avatar image above the first of a chain of messages.
shpuld commented 2020-05-07 18:34:53 +00:00
Owner
Copy link

counterexamples that are not famous for foss tier ui: whatsapp and telegram

still let's add the information

counterexamples that are not famous for foss tier ui: whatsapp and telegram still let's add the information
hj commented 2020-05-07 18:35:30 +00:00
Owner
Copy link

I can say that iMessages hide avatars for both sides in 1-on-1s and Skype hides avatars for outgoing messages.

I would prefer avatars on incoming messages on by default and off for outgoing, probably make it configurable but it's not important right now

I can say that iMessages hide avatars for both sides in 1-on-1s and Skype hides avatars for outgoing messages. I would prefer avatars on incoming messages on by default and off for outgoing, probably make it configurable but it's not important right now
shpuld commented 2020-05-07 18:41:54 +00:00
Owner
Copy link

vote yes for 'no avatars or names on outgoing messages', vote big no for making incoming message sender info into an option, we need those avatars for group chats later anyway so don't bother making 2 different implementations, and especially don't add to the clutter in our options

vote yes for 'no avatars or names on outgoing messages', vote big no for making incoming message sender info into an option, we need those avatars for group chats later anyway so don't bother making 2 different implementations, and especially don't add to the clutter in our options
lambadalambda commented 2020-05-07 18:48:59 +00:00
Owner
Copy link

my vote is also for 'incoming: yes, outgoing: no', with no options.

my vote is also for 'incoming: yes, outgoing: no', with no options.
lambadalambda commented 2020-05-07 18:59:27 +00:00
Owner
Copy link

I quite like the way it's done in LINE, very unobtrusive

UoLuUyuEZuiQGxQnkuBekQwW

I quite like the way it's done in LINE, very unobtrusive ![UoLuUyuEZuiQGxQnkuBekQwW](/attachments/79ed1fa5-8a88-401e-8473-cf2a23785e16)
UoLuUyuEZuiQGxQnkuBekQwW.png
107 KiB
shpuld commented 2020-05-07 19:08:55 +00:00
Owner
Copy link

Skype is mostly the same but also adds the name on top with a smaller fainter font, which is maybe not that useful for 1on1 chats but borderline must for group chats later

Skype is mostly the same but also adds the name on top with a smaller fainter font, which is maybe not that useful for 1on1 chats but borderline must for group chats later
hj commented 2020-05-07 19:17:07 +00:00
Owner
Copy link

lucky cute dogs

lucky cute dogs
eugenijm commented 2020-05-07 19:31:15 +00:00
Author
Member
Copy link

added avatars for incoming messages

added avatars for incoming messages
lambadalambda commented 2020-05-08 09:36:41 +00:00
Owner
Copy link

much nicer now

Screenshot_2020-05-08_dontbulling_me

much nicer now ![Screenshot_2020-05-08_dontbulling_me](/attachments/b8ba44d0-fe7a-4311-94ca-2de1f396e69d)
Screenshot_2020-05-08_dontbulling_me.png
285 KiB
shpuld commented 2020-05-08 09:43:32 +00:00
Owner
Copy link

some UI nitpicks based on that screenshot, the avatar isn't lined up well, lining up with the bottom of the message looks weird. either top or slightly off from top (to make it look centered on 1-line messages but not on longer ones) would be better

also now it looks a bit strange with the recipient avatar in the upper right corner, maybe the avatar could be together with the name in the center, right before it

some UI nitpicks based on that screenshot, the avatar isn't lined up well, lining up with the bottom of the message looks weird. either top or slightly off from top (to make it look centered on 1-line messages but not on longer ones) would be better also now it looks a bit strange with the recipient avatar in the upper right corner, maybe the avatar could be together with the name in the center, right before it
hj commented 2020-05-08 16:32:48 +00:00
Owner
Copy link

avatar should be aligned with top line of first message i'd say.

avatar in titlebar... weh. I don't think it's required at all for 1-on-1s at all, but should be a thing for group chats in the future. Probably should be on the left side of title, either next to it or on the far left side. Right side is ok i guess? That's the place where menu would normally be tho...

avatar should be aligned with top line of first message i'd say. avatar in titlebar... weh. I don't think it's required at all for 1-on-1s at all, but should be a thing for group chats in the future. Probably should be on the left side of title, either next to it or on the far left side. Right side is ok i guess? That's the place where menu would normally be tho...
lambadalambda commented 2020-05-11 09:27:33 +00:00
Owner
Copy link

After trying it out some more, here are a few things that don't seem right:

  • I think pressing enter should post the chat message
  • Scrolling up will not load in old chat messages
  • Opening the emoji picker will look quite weird and make the chat window jump around, see pic
  • A chat message notification should probably not show up in the usual notification box, but instead just count up an unread messages indicator on the 'chat' link in the navbar
  • custom emoji aren't handled at the moment
  • attachments aren't handled at the moment
  • last_message isn't handled at the moment

It's already fun to use, though :)

Screenshot_2020-05-11_originalpatchou_li

After trying it out some more, here are a few things that don't seem right: - I think pressing enter should post the chat message - Scrolling up will not load in old chat messages - Opening the emoji picker will look quite weird and make the chat window jump around, see pic - A chat message notification should probably not show up in the usual notification box, but instead just count up an unread messages indicator on the 'chat' link in the navbar - custom emoji aren't handled at the moment - attachments aren't handled at the moment - last_message isn't handled at the moment It's already fun to use, though :) ![Screenshot_2020-05-11_originalpatchou_li](/attachments/a3aa2f4a-a344-4fc5-8347-c5c9009a7eab)
Screenshot_2020-05-11_originalpatchou_li.png
749 KiB
lambadalambda commented 2020-05-11 09:54:32 +00:00
Owner
Copy link

One more:

  • chats are getting reordered like half a second after opening the chat list, but they don't need to, they are already in the 'most recently updated' order.
One more: - chats are getting reordered like half a second after opening the chat list, but they don't need to, they are already in the 'most recently updated' order.
eugenijm commented 2020-05-11 10:40:57 +00:00
Author
Member
Copy link

For comparison, the screenshots with the avatar and the nickname in the title:

  1. in the center
  2. on the left

With regards to the avatar and message alignment, we can line up 1) the top of avatar with the top of the first line 2) the middle of the avatar with the middle of the first line 3) the top of the avatar with the the top of the message bubble / cloud

For comparison, the screenshots with the avatar and the nickname in the title: 1) [in the center](/attachments/90e40dd3-5e7d-4cc0-83b2-ecbd5d84f8af) 2) [on the left](/attachments/3a757ce9-4acc-4ae8-937d-e5a22af0caba) With regards to the avatar and message alignment, we can line up 1) the top of avatar with the top of the first line 2) the middle of the avatar with the middle of the first line 3) the top of the avatar with the the top of the message bubble / cloud
layout-title-in-the-center.png
407 KiB
layout-title-on-the-left.png
407 KiB
hj commented 2020-05-11 12:14:36 +00:00
Owner
Copy link

i think pressing enter

already mentioned above

custom emoji aren't handled at the moment

wym?

>i think pressing enter already mentioned above >custom emoji aren't handled at the moment wym?
hj commented 2020-05-11 12:17:06 +00:00
Owner
Copy link

avatar and nick name should be on the left side, i just realized that's how titlebar titles work in every other place in the UI.

for message alignment - top of avatar aligned with top of the bubble.

avatar and nick name should be on the left side, i just realized that's how titlebar titles work in every other place in the UI. for message alignment - top of avatar aligned with top of the bubble.
lambadalambda commented 2020-05-11 13:48:27 +00:00
Owner
Copy link

Screenshot_2020-05-11_originalpatchou_li_1_

this :)

![Screenshot_2020-05-11_originalpatchou_li_1_](/attachments/df746e2f-588c-4233-8ff7-dc345ba7f259) this :)
Screenshot_2020-05-11_originalpatchou_li_1_.png
55 KiB
lambadalambda commented 2020-05-11 13:49:24 +00:00
Owner
Copy link

yeah, the left alignment looks much nicer

yeah, the left alignment looks much nicer
hj commented 2020-05-11 15:01:03 +00:00
Owner
Copy link

oh. right. emojis are added at API layer not in the component, forgot.

oh. right. emojis are added at API layer not in the component, forgot.
eugenijm commented 2020-05-11 22:41:20 +00:00
Author
Member
Copy link

Added last_message to the chat list, attachments, custom emoji, message loading on scroll up, unread count indicator (hid chat messages from the notification box), fixed the chats reordering

Added last_message to the chat list, attachments, custom emoji, message loading on scroll up, unread count indicator (hid chat messages from the notification box), fixed the chats reordering
lambadalambda commented 2020-05-12 07:56:33 +00:00
Owner
Copy link

Current version is pretty nice, but I found a new problem:

Screenshot_2020-05-12_originalpatchou_li_1_

Current version is pretty nice, but I found a new problem: ![Screenshot_2020-05-12_originalpatchou_li_1_](/attachments/1dcbc2d5-3b6c-4166-a978-f84218bd08e8)
Screenshot_2020-05-12_originalpatchou_li_1_.png
18 KiB
lambadalambda commented 2020-05-12 07:56:59 +00:00
Owner
Copy link

Screenshot_2020-05-12_dontbulling_me

attachments look great!

![Screenshot_2020-05-12_dontbulling_me](/attachments/5b73ca57-0baf-4332-9785-f9ebc8ef975a) attachments look great!
Screenshot_2020-05-12_dontbulling_me.png
438 KiB
eugenijm commented 2020-05-12 08:45:58 +00:00
Author
Member
Copy link

oh, fixed it

oh, fixed it
lambadalambda commented 2020-05-13 09:39:51 +00:00
Owner
Copy link

I experienced some weirdness with attachments, sometimes the whole post doesn't show up. It seems to happen mainly with remote attachments.

Screenshot_2020-05-12_originalpatchou_li_3_

Screenshot_2020-05-12_originalpatchou_li_2_

Screenshot_from_2020-05-12_19-16-30

I experienced some weirdness with attachments, sometimes the whole post doesn't show up. It seems to happen mainly with remote attachments. ![Screenshot_2020-05-12_originalpatchou_li_3_](/attachments/1b3d5bfc-ab8f-423d-9483-f12e796718d0) ![Screenshot_2020-05-12_originalpatchou_li_2_](/attachments/171f98e3-0bbf-4ac1-8d6b-81e8f147f6d8) ![Screenshot_from_2020-05-12_19-16-30](/attachments/4c8d31f6-2c69-4afa-86f9-6124236ae784)
Screenshot_2020-05-12_originalpatchou_li_3_.png
46 KiB
Screenshot_2020-05-12_originalpatchou_li_2_.png
341 KiB
Screenshot_from_2020-05-12_19-16-30.png
132 KiB
lambadalambda commented 2020-05-13 13:37:40 +00:00
Owner
Copy link

also, chat messages can be posted now without content, if there's an attachment

also, chat messages can be posted now without content, if there's an attachment
eugenijm commented 2020-05-13 14:28:00 +00:00
Author
Member
Copy link

fixed it in the last update, remote attachments work now

fixed it in the last update, remote attachments work now
eugenijm commented 2020-05-13 14:32:55 +00:00
Author
Member
Copy link

nice, removed the client error when the attachment is without content

nice, removed the client error when the attachment is without content
lambadalambda commented 2020-05-14 08:59:54 +00:00
Owner
Copy link

tested, works :)

tested, works :)
lambadalambda commented 2020-05-14 09:00:00 +00:00
Owner
Copy link

tested, works :)

tested, works :)
lambadalambda commented 2020-05-14 10:25:50 +00:00
Owner
Copy link

A few more things:

  • last_message doesn't change without refresh
  • i'll add an 'updated_at' column to chats, they are already sorted by this but addind it will make it possible to sort on the frontend as well and keep things sorted as new messages arrive.
A few more things: - `last_message` doesn't change without refresh - i'll add an 'updated_at' column to chats, they are already sorted by this but addind it will make it possible to sort on the frontend as well and keep things sorted as new messages arrive.
lambadalambda commented 2020-05-14 12:00:45 +00:00
Owner
Copy link

Oh, and if the last message is just an attachment, it won't display any text in the chat list

Oh, and if the last message is just an attachment, it won't display any text in the chat list
lanodan commented 2020-05-14 22:55:07 +00:00
Owner
Copy link

Updated pleroma-fe to develop plus this branch today (with also updating the backend) and I got this… I wish there would be an actual error (backend usually provides an error message). screen

Also tried it in a brand new session, same result.

Updated pleroma-fe to develop plus this branch today (with also updating the backend) and I got this… I wish there would be an actual error (backend usually provides an error message). ![screen](/attachments/ba620ed6-5a07-4fb5-91a3-47686f37cbe3) Also tried it in a brand new session, same result.
screen.png
16 KiB
lambadalambda commented 2020-05-14 23:28:14 +00:00
Owner
Copy link

maybe stupid question, but do you run the remake-remodel-dms backend branch?

maybe stupid question, but do you run the remake-remodel-dms backend branch?
lanodan commented 2020-05-15 06:22:55 +00:00
Owner
Copy link

Nah, develop plus some of my MRs…

Nah, develop plus some of my MRs…
eugenijm commented 2020-05-15 06:36:07 +00:00
Author
Member
Copy link

Fixed those in the last update. last_message is kept updated, the chat list is kept sorted by updated_at, the info about the attachment file type (image, video, etc) is shown in the chat list.

Fixed those in the last update. `last_message` is kept updated, the chat list is kept sorted by `updated_at`, the info about the attachment file type (image, video, etc) is shown in the chat list.
lambadalambda commented 2020-05-15 08:47:13 +00:00
Owner
Copy link

well, than you don't have the chat endpoints

well, than you don't have the chat endpoints
shpuld commented 2020-05-21 11:37:35 +00:00
Owner
Copy link

tested a bit, I feel like the chat stuff is in good hands, but a few small things:

  • something awkward about the ellipsis button I feel. especially on mobile you need a tap to show the button (not super intuitive), then tap the button, then you get the option to delete, then a confirmation if you wanted to delete, that's 4 taps for one action. I'll try to figure out some design for it

  • the avatar/name on top looks like it's not really aligned to anything at all. not sure why the centered was changed, but I think I'll try out some things with it

  • does not behave well with on screen keyboard, see how I have to manually scroll to bottom multiple times: https://shpposter.club/media/7c8eda01e9ce9133f3b166efb18b9ca14f0148d245314009810969baa0b6974f.mp4

tested a bit, I feel like the chat stuff is in good hands, but a few small things: - something awkward about the ellipsis button I feel. especially on mobile you need a tap to show the button (not super intuitive), then tap the button, then you get the option to delete, then a confirmation if you wanted to delete, that's 4 taps for one action. I'll try to figure out some design for it - the avatar/name on top looks like it's not really aligned to anything at all. not sure why the centered was changed, but I think I'll try out some things with it - does not behave well with on screen keyboard, see how I have to manually scroll to bottom multiple times: https://shpposter.club/media/7c8eda01e9ce9133f3b166efb18b9ca14f0148d245314009810969baa0b6974f.mp4
shpuld commented 2020-05-21 11:52:17 +00:00
Owner
Copy link

oh it also lets you upload multiple images but only 1 will post

oh it also lets you upload multiple images but only 1 will post
shpuld commented 2020-05-21 12:06:02 +00:00
Owner
Copy link

also, what's the small (i) button in the top corner for?

also, what's the small (i) button in the top corner for?
eugenijm commented 2020-05-21 13:29:58 +00:00
Author
Member
Copy link

I thought we could use it in the future to link it to a potential chat control page where the user could mute or delete the chat (although we can also use the avatar/name in the header for this purpose). I guess it would be better to remove it for now until we need something like this to avoid confusion

I thought we could use it in the future to link it to a potential chat control page where the user could mute or delete the chat (although we can also use the avatar/name in the header for this purpose). I guess it would be better to remove it for now until we need something like this to avoid confusion
eugenijm commented 2020-05-21 13:38:13 +00:00
Author
Member
Copy link

fixed!

fixed!
eugenijm commented 2020-05-22 22:28:48 +00:00
Author
Member
Copy link

the on screen keyboard behavior should be better now (also disabled autofocus on mobile that brings the keyboard) (edit: will try to make it so that OSK stays in the same position after message send instead of disappearing)

the on screen keyboard behavior should be better now (also disabled autofocus on mobile that brings the keyboard) (edit: will try to make it so that OSK stays in the same position after message send instead of disappearing)
lambadalambda commented 2020-05-25 15:59:49 +00:00
Owner
Copy link

testing the latest release, working quite well! With a activated notification streaming, the chat is even in real time :)

testing the latest release, working quite well! With a activated notification streaming, the chat is even in real time :)
lambadalambda commented 2020-05-25 18:05:53 +00:00
Owner
Copy link

ignore

~~ignore~~
lambadalambda commented 2020-05-26 11:48:55 +00:00
Owner
Copy link

Also, can you change the sending behavior so that

  1. Pressing enter will send the message
  2. Pressing shift+enter will start a new line
Also, can you change the sending behavior so that 1. Pressing enter will send the message 2. Pressing shift+enter will start a new line
lambadalambda commented 2020-05-26 12:03:05 +00:00
Owner
Copy link

and somehow the time got doubled here

Screenshot_2020-05-26_dontbulling_me

and somehow the time got doubled here ![Screenshot_2020-05-26_dontbulling_me](/attachments/6ad2c05f-806b-4eb8-956e-ba2ee635bd9d)
Screenshot_2020-05-26_dontbulling_me.png
20 KiB
eugenijm commented 2020-05-26 19:35:13 +00:00
Author
Member
Copy link

fixed

fixed
eugenijm commented 2020-05-26 19:41:42 +00:00
Author
Member
Copy link

done

done
hj commented 2020-05-26 19:52:43 +00:00
Owner
Copy link

i protest!! put it back!!

i protest!! put it back!!
shpuld commented 2020-05-26 21:09:23 +00:00
Owner
Copy link

ctrl enter to send chat messages is awkward as hell, I expect enter to send a message like 99% of chat applications

ctrl enter to send chat messages is awkward as hell, I expect enter to send a message like 99% of chat applications
hj commented 2020-05-26 21:12:48 +00:00
Owner
Copy link

that's a personal preference and context-sensitive.

post form is nearly identical to other post forms, i'd expect it to behave the same way as well.

either

a) add "Enter sends, use shift-enter to add newlines" hint somewhere
b) add option to configure it
c) both

that's a personal preference and context-sensitive. post form is nearly identical to other post forms, i'd expect it to behave the same way as well. either a) add "Enter sends, use shift-enter to add newlines" hint somewhere b) add option to configure it c) both
hj commented 2020-05-26 21:13:06 +00:00
Owner
Copy link

also what's up with missing polls button in form?

also what's up with missing polls button in form?
shpuld commented 2020-05-26 21:14:47 +00:00
Owner
Copy link

it's really not the same at all, the form doesn't make the difference, but the context does.

it's really not the same at all, the form doesn't make the difference, but the context does.
shpuld commented 2020-05-27 14:06:45 +00:00
Owner
Copy link

chat messages are not statuses, they don't have polls

chat messages are not statuses, they don't have polls
shpuld commented 2020-05-27 14:07:39 +00:00
Owner
Copy link

some bugs with the enter-to-send: ctrl-enter should still send too, and it shouldn't send when you hit it to insert an emoji

some bugs with the enter-to-send: ctrl-enter should still send too, and it shouldn't send when you hit it to insert an emoji
lambadalambda commented 2020-05-27 14:52:56 +00:00
Owner
Copy link

Screenshot_2020-05-27_dontbulling_me_1_

seems like there's some reactivity issue with the user here

![Screenshot_2020-05-27_dontbulling_me_1_](/attachments/15375a90-33b6-4814-88fd-48f9e03cefb3) seems like there's some reactivity issue with the user here
Screenshot_2020-05-27_dontbulling_me_1_.png
8.5 KiB
hj commented 2020-05-27 17:10:02 +00:00
Owner
Copy link

image tag needs key=.. to make entire tag rerender, maybe?

image tag needs `key=..` to make entire tag rerender, maybe?
hj commented 2020-05-27 17:10:28 +00:00
Owner
Copy link

polls are fairly useful in group chats tho.

polls are fairly useful in group chats tho.
shpuld commented 2020-05-27 17:19:06 +00:00
Owner
Copy link

that's a feature request for the future then

that's a feature request for the future then
shpuld commented 2020-05-27 17:19:27 +00:00
Owner
Copy link

probably noticed already, but the emoji autocomplete goes off-screen

probably noticed already, but the emoji autocomplete goes off-screen
eugenijm commented 2020-05-28 16:28:55 +00:00
Author
Member
Copy link

fixed ctrl-enter and the emoji insertion

fixed ctrl-enter and the emoji insertion
lambadalambda commented 2020-05-29 14:26:32 +00:00
Owner
Copy link

For streaming, i added a new user:pleroma_chat stream, but the mssage also comes through in user. The incoming payload on a new chat message has the event pleroma:chat_update, the payload is the updated chat, the message is in the last_message field. Please use this to add chat messages and don't add them from notifications anymore.

For streaming, i added a new `user:pleroma_chat` stream, but the mssage also comes through in `user`. The incoming payload on a new chat message has the event `pleroma:chat_update`, the payload is the updated chat, the message is in the `last_message` field. Please use this to add chat messages and don't add them from notifications anymore.
lambadalambda commented 2020-05-29 14:26:43 +00:00
Owner
Copy link

This is live on dontbulling.me

This is live on dontbulling.me
lambadalambda commented 2020-05-29 14:38:05 +00:00
Owner
Copy link

A few more observations:

  • Chats currently don't generate any change in the title of the site, because they don't increase the notification count.
  • When in a chat, the constant pull of /messages doesn't seem to use since_id. While this doesn't hurt much because chat messages are so small, it is a waste of bandwidth and server time.
  • When streaming is used, pulling can be disabled completely because messages will come in through streaming. If connectivity is lost, however, an initial clearing and re-pull of the chat has to be done, if any is open at the moment.
A few more observations: - Chats currently don't generate any change in the title of the site, because they don't increase the notification count. - When in a chat, the constant pull of `/messages` doesn't seem to use `since_id`. While this doesn't hurt much because chat messages are so small, it is a waste of bandwidth and server time. - When streaming is used, pulling can be disabled completely because messages will come in through streaming. If connectivity is lost, however, an initial clearing and re-pull of the chat has to be done, if any is open at the moment.
lambadalambda commented 2020-05-30 14:57:35 +00:00
Owner
Copy link

this one is not an event, it's a possible value for stream, like user or public

this one is not an event, it's a possible value for `stream`, like `user` or `public`
lambadalambda commented 2020-05-30 17:11:59 +00:00
Owner
Copy link

newlines in normal posts broke

newlines in normal posts broke
eugenijm commented 2020-05-30 17:30:04 +00:00
Author
Member
Copy link

fixed

fixed
eugenijm commented 2020-05-30 17:30:35 +00:00
Author
Member
Copy link

done

done
lambadalambda commented 2020-06-02 14:07:23 +00:00
Owner
Copy link

When you have a chat open but not focused (that is, you're on a different tab), you will never see the number rise in the title because the focused chat will immediately send a 'read' as soon as it receives a message. It should only do that when it's in focus or becoming in focus.

When you have a chat open but not focused (that is, you're on a different tab), you will never see the number rise in the title because the focused chat will immediately send a 'read' as soon as it receives a message. It should only do that when it's in focus or becoming in focus.
eugenijm commented 2020-06-02 16:55:38 +00:00
Author
Member
Copy link

Fixed (added focus tracking)

Fixed (added focus tracking)
eugenijm commented 2020-06-02 17:25:39 +00:00
Author
Member
Copy link

I guess it would be handy to track idle status too (mouse movements / keystrokes) so that we don't mark new messages as read when the user is most likely away from the keyboard despite having a focused chat tab, will add this

I guess it would be handy to track idle status too (mouse movements / keystrokes) so that we don't mark new messages as read when the user is most likely away from the keyboard despite having a focused chat tab, will add this
eugenijm commented 2020-06-02 17:32:34 +00:00
Author
Member
Copy link

fixed

fixed
lambadalambda commented 2020-06-03 11:24:37 +00:00
Owner
Copy link

The chat breaks (only showing max 1 message) when the message is not a number. Both the chat id and the chat message ids are strings and should be treated as such. The chat message ids are sortable.

https://originalpatchou.li/ is running with chat message ids like 9vhjDjbSBxzScg77Ro

The chat breaks (only showing max 1 message) when the message is not a number. Both the chat id and the chat message ids are strings and should be treated as such. The chat message ids are sortable. https://originalpatchou.li/ is running with chat message ids like 9vhjDjbSBxzScg77Ro
lambadalambda commented 2020-06-03 11:24:55 +00:00
Owner
Copy link

Also on that branch, any message can be deleted, not just your own.

Also on that branch, any message can be deleted, not just your own.
hj commented 2020-06-03 11:38:25 +00:00
Owner
Copy link

finally

finally
lambadalambda commented 2020-06-03 11:41:59 +00:00
Owner
Copy link

And one more thing, refreshing a chat (like https://dontbulling.me/users/lain/chats/9u6Qw542D2VvvcSOQq) or going there directly won't actually show any messages

And one more thing, refreshing a chat (like https://dontbulling.me/users/lain/chats/9u6Qw542D2VvvcSOQq) or going there directly won't actually show any messages
lambadalambda commented 2020-06-03 12:47:31 +00:00
Owner
Copy link

I don't understand why this is necessary. The string ids are already sortable.

I don't understand why this is necessary. The string ids are already sortable.
lambadalambda commented 2020-06-03 12:52:38 +00:00
Owner
Copy link

Messages incoming via stream don't update the open chat properly anymore

Messages incoming via stream don't update the open chat properly anymore
eugenijm commented 2020-06-03 13:08:27 +00:00
Author
Member
Copy link

Troubleshooting this, looks like FE is getting the previous message in pleroma:chat_update instead of the last one. (since the previous message is already in the store, the chat isn't updated). E.g., when the user sends 1, 2, 3, then receiver gets 1 when 2 is sent and 2 when 3 is sent.

Troubleshooting this, looks like FE is getting the previous message in `pleroma:chat_update` instead of the last one. (since the previous message is already in the store, the chat isn't updated). E.g., when the user sends 1, 2, 3, then receiver gets 1 when 2 is sent and 2 when 3 is sent.
lambadalambda commented 2020-06-03 13:16:46 +00:00
Owner
Copy link

ah, i'm checking!

ah, i'm checking!
lanodan commented 2020-06-03 14:12:10 +00:00
Owner
Copy link

There is huge issues with this kind of tracking:

  • mouse mouvements can be very untrustworthy (cat/kid, mouse on a weird surface or broken touchpad)
  • you could have the window be focused for few seconds or minutes but be actually looking away
  • in some setups you still have keys/mouse leaking to other windows when there isn't any focus

Right now we have an explicit read button for notifications and I think it's very good.

There is huge issues with this kind of tracking: - mouse mouvements can be very untrustworthy (cat/kid, mouse on a weird surface or broken touchpad) - you could have the window be focused for few seconds or minutes but be actually looking away - in some setups you still have keys/mouse leaking to other windows when there isn't any focus Right now we have an explicit read button for notifications and I think it's very good.
lambadalambda commented 2020-06-03 14:53:12 +00:00
Owner
Copy link

Thank you, found the problem and fixed it.

Thank you, found the problem and fixed it.
eugenijm commented 2020-06-03 17:06:15 +00:00
Author
Member
Copy link

The chat breaks (only showing max 1 message) when the message is not a number.

This is fixed now, when investigating I found that the connection was closing randomly on some messages and FE wasn't handling the WS connection loss properly. I fixed the WS connection loss handling, however, the somewhat random and infrequent connection loss on new messages seem to remain. Made a video to illustrate it (as soon as the connection is closed, the new one is created instead and we do the clean up and re-pull as per #2286) (this happens on the receiver's side)

the related error in the logs

[error] Ranch listener Pleroma.Web.Endpoint.HTTP had connection process started with :cowboy_clear:start_link/4 at #PID<0.2207.0> exit with reason: {%Phoenix.Template.UndefinedError{assigns: %{chat_message_reference: nil, for: %Pleroma.User{emoji: %{}, multi_factor_authentication_settings: %Pleroma.MFA.Settings{backup_codes: [], enabled: false, totp: nil}, blocked_users: #Ecto.Association.NotLoaded<association :blocked_users is not loaded>, password_reset_pending: false, notification_mutee_mutes: #Ecto.Association.NotLoaded<association :notification_mutee_mutes is not loaded>, ap_enabled: false, email_notifications: %{"digest" => false}, fields: [], search_rank: nil, locked: false, reblog_muted_users: #Ecto.Association.NotLoaded<association :reblog_muted_users is not loaded>, inbox: nil, discoverable: false, subscribee_users: #Ecto.Association.NotLoaded<association :subscribee_users is not loaded>, search_type: nil, ap_id: "http://localhost:4000/users/pyrosome", __meta__: #Ecto.Schema.Metadata<:loaded, "users">, skip_thread_containment: false, confirmation_token: nil, deactivated: false, reblog_muter_users: #Ecto.Association.NotLoaded<association :reblog_muter_users is not loaded>, follower_count: 1, banner: %{}, uri: nil, invisible: false, unread_conversation_count: 2, muted_users: #Ecto.Association.NotLoaded<association :muted_users is not loaded>, notification_settings: %Pleroma.User.NotificationSetting{followers: true, follows: true, non_followers: true, non_follows: true, privacy_option: false}, blocker_users: #Ecto.Association.NotLoaded<association :blocker_users is not loaded>, following_count: 1, also_known_as: [], notification_muter_users: #Ecto.Association.NotLoaded<association :notification_muter_users is not loaded>, notifications: #Ecto.Association.NotLoaded<association :notifications is not loaded>, nickname: "pyrosome", reblog_muter_mutes: #Ecto.Association.NotLoaded<association :reblog_muter_mutes is not loaded>, incoming_relationships: #Ecto.Association.NotLoaded<association :incoming_relationships is not loaded>, inserted_at: ~N[2020-03-21 01:36:51], hide_favorites: true, notification_muter_mutes: #Ecto.Association.NotLoaded<association :notification_muter_mutes is not loaded>, no_rich_text: false, name: "pyrosome", follower_address: "http://localhost:4000/users/pyrosome/followers", reblog_mutee_mutes: #Ecto.Association.NotLoaded<association :reblog_mutee_mutes is not loaded>, confirmation_pending: false, last_refreshed_at: nil, pinned_activities: [], ...}, relationships: nil}, available: [], module: Pleroma.Web.PleromaAPI.ChatMessageReferenceView, pattern: "*", root: "lib/pleroma/web/templates/pleroma_api/chat_message_reference", template: "show.json"}, [

  {Phoenix.Template, :raise_template_not_found, 3, [file: 'lib/phoenix/template.ex', line: 341]}, 
  {Pleroma.Web.MastodonAPI.NotificationView, :put_chat_message, 4, [file: 'lib/pleroma/web/mastodon_api/views/notification_view.ex', line: 154]}, 
  {Pleroma.Web.StreamerView, :render, 3, [file: 'lib/pleroma/web/views/streamer_view.ex', line: 32]}, 
  {Pleroma.Web.MastodonAPI.WebsocketHandler, :websocket_info, 2, [file: 'lib/pleroma/web/mastodon_api/websocket_handler.ex', line: 77]}, 
  {:cowboy_websocket, :handler_call, 6, [file: '/home/asdf/code/pleroma/deps/cowboy/src/cowboy_websocket.erl', line: 482]}, 
  {:cowboy_http, :loop, 1, [file: '/home/asdf/code/pleroma/deps/cowboy/src/cowboy_http.erl', line: 231]}, {:proc_lib, :init_p_do_apply, 3, [file: 'proc_lib.erl', line: 249]}]}
> The chat breaks (only showing max 1 message) when the message is not a number. This is fixed now, when investigating I found that the connection was closing randomly on some messages and FE wasn't handling the WS connection loss properly. I fixed the WS connection loss handling, however, the somewhat random and infrequent connection loss on new messages seem to remain. Made a [video](/attachments/b145157a-9fc4-44db-bf8f-701c509a99ad) to illustrate it (as soon as the connection is closed, the new one is created instead and we do the clean up and re-pull as per https://git.pleroma.social/pleroma/pleroma-fe/pulls/2286#note_61521) (this happens on the receiver's side) the related error in the logs ``` [error] Ranch listener Pleroma.Web.Endpoint.HTTP had connection process started with :cowboy_clear:start_link/4 at #PID<0.2207.0> exit with reason: {%Phoenix.Template.UndefinedError{assigns: %{chat_message_reference: nil, for: %Pleroma.User{emoji: %{}, multi_factor_authentication_settings: %Pleroma.MFA.Settings{backup_codes: [], enabled: false, totp: nil}, blocked_users: #Ecto.Association.NotLoaded<association :blocked_users is not loaded>, password_reset_pending: false, notification_mutee_mutes: #Ecto.Association.NotLoaded<association :notification_mutee_mutes is not loaded>, ap_enabled: false, email_notifications: %{"digest" => false}, fields: [], search_rank: nil, locked: false, reblog_muted_users: #Ecto.Association.NotLoaded<association :reblog_muted_users is not loaded>, inbox: nil, discoverable: false, subscribee_users: #Ecto.Association.NotLoaded<association :subscribee_users is not loaded>, search_type: nil, ap_id: "http://localhost:4000/users/pyrosome", __meta__: #Ecto.Schema.Metadata<:loaded, "users">, skip_thread_containment: false, confirmation_token: nil, deactivated: false, reblog_muter_users: #Ecto.Association.NotLoaded<association :reblog_muter_users is not loaded>, follower_count: 1, banner: %{}, uri: nil, invisible: false, unread_conversation_count: 2, muted_users: #Ecto.Association.NotLoaded<association :muted_users is not loaded>, notification_settings: %Pleroma.User.NotificationSetting{followers: true, follows: true, non_followers: true, non_follows: true, privacy_option: false}, blocker_users: #Ecto.Association.NotLoaded<association :blocker_users is not loaded>, following_count: 1, also_known_as: [], notification_muter_users: #Ecto.Association.NotLoaded<association :notification_muter_users is not loaded>, notifications: #Ecto.Association.NotLoaded<association :notifications is not loaded>, nickname: "pyrosome", reblog_muter_mutes: #Ecto.Association.NotLoaded<association :reblog_muter_mutes is not loaded>, incoming_relationships: #Ecto.Association.NotLoaded<association :incoming_relationships is not loaded>, inserted_at: ~N[2020-03-21 01:36:51], hide_favorites: true, notification_muter_mutes: #Ecto.Association.NotLoaded<association :notification_muter_mutes is not loaded>, no_rich_text: false, name: "pyrosome", follower_address: "http://localhost:4000/users/pyrosome/followers", reblog_mutee_mutes: #Ecto.Association.NotLoaded<association :reblog_mutee_mutes is not loaded>, confirmation_pending: false, last_refreshed_at: nil, pinned_activities: [], ...}, relationships: nil}, available: [], module: Pleroma.Web.PleromaAPI.ChatMessageReferenceView, pattern: "*", root: "lib/pleroma/web/templates/pleroma_api/chat_message_reference", template: "show.json"}, [ {Phoenix.Template, :raise_template_not_found, 3, [file: 'lib/phoenix/template.ex', line: 341]}, {Pleroma.Web.MastodonAPI.NotificationView, :put_chat_message, 4, [file: 'lib/pleroma/web/mastodon_api/views/notification_view.ex', line: 154]}, {Pleroma.Web.StreamerView, :render, 3, [file: 'lib/pleroma/web/views/streamer_view.ex', line: 32]}, {Pleroma.Web.MastodonAPI.WebsocketHandler, :websocket_info, 2, [file: 'lib/pleroma/web/mastodon_api/websocket_handler.ex', line: 77]}, {:cowboy_websocket, :handler_call, 6, [file: '/home/asdf/code/pleroma/deps/cowboy/src/cowboy_websocket.erl', line: 482]}, {:cowboy_http, :loop, 1, [file: '/home/asdf/code/pleroma/deps/cowboy/src/cowboy_http.erl', line: 231]}, {:proc_lib, :init_p_do_apply, 3, [file: 'proc_lib.erl', line: 249]}]} ```
ws-connection-loss.mp4
807 KiB
eugenijm commented 2020-06-03 17:36:18 +00:00
Author
Member
Copy link

fixed the emoji autocomplete going off-screen

fixed the emoji autocomplete going off-screen
lambadalambda commented 2020-06-03 17:44:39 +00:00
Owner
Copy link

a backend bug that should be fixed now

a backend bug that should be fixed now
lambadalambda commented 2020-06-03 17:45:08 +00:00
Owner
Copy link

Still good that you wrote the code to reestablish, thank you!

Still good that you wrote the code to reestablish, thank you!
shpuld commented 2020-06-03 19:16:49 +00:00
Owner
Copy link

I don't think it makes sense to keep track of unread posts in currently focused conversation, you can only have one. I don't think most IM applications do anything like this either. I think it's only important to check if the tab is focused

I don't think it makes sense to keep track of unread posts in currently focused conversation, you can only have one. I don't think most IM applications do anything like this either. I think it's only important to check if the tab is focused
shpuld commented 2020-06-03 19:35:24 +00:00
Owner
Copy link

the title bar change when unfocused is working very unreliable, more than half of the times when I leave the tab, I don't get any change in the title bar to tell me there's new messages

the title bar change when unfocused is working very unreliable, more than half of the times when I leave the tab, I don't get any change in the title bar to tell me there's new messages
eugenijm commented 2020-06-04 09:23:09 +00:00
Author
Member
Copy link

Noticed a similar issue with the unread value in the pleroma:chat_update payload: the unread value doesn't include the new unread message (right now this is not noticeable on FE because it re-fetches the chat on a new message but it would prob. be better to remove that extra fetch and use the pleroma:chat_update payload instead). (BE version: e46aecda55b20c0d48463fb2a5c0040d4fc34e97, also confirmed it happens on dontbulling.me too)

Noticed a similar issue with the unread value in the `pleroma:chat_update` payload: the `unread` value doesn't include the new unread message (right now this is not noticeable on FE because it re-fetches the chat on a new message but it would prob. be better to remove that extra fetch and use the `pleroma:chat_update` payload instead). (BE version: `e46aecda55b20c0d48463fb2a5c0040d4fc34e97`, also confirmed it happens on dontbulling.me too)
lambadalambda commented 2020-06-04 11:17:07 +00:00
Owner
Copy link

Thank you for reporting this, this has been fixed, the unread count is accurate now

Thank you for reporting this, this has been fixed, the unread count is accurate now
eugenijm commented 2020-06-04 13:43:31 +00:00
Author
Member
Copy link

Still happens on the latest remake-remodel-dms branch (the unread doesn't contain the updated value): video (in the video, I disabled the chat re-refetch on a new message so that we always display the unread count received from pleroma:chat_update: the left user sends "1", the receiver gets it but the unread count is still 0, then the WS connection drops on the second message and we restore the counter on reconnect).

This is a bit strange, since the unread count is recalculated in the chat view. It looks like the db query gets a different unread count when streaming out the pleroma:chat_update event, that doesn't include the newly created message.

I suspect this might be related to the fact that Pipeline.common_pipeline/2 runs in a transaction, and then inside the transaction we are spawning an async process in Stream.do_stream, and onside the DB transaction, the db query calculates a different unread count.

I tried to verify this locally by removing the transaction wrapper in the Pipeline.common_pipeline call and it fixed the issue for me.

By the way, I updated this branch so that it uses the unread value received from pleroma:chat_update (instead of re-fetching the chat on a new message), so the issue should be more visible now.

Still happens on the latest `remake-remodel-dms` branch (the unread doesn't contain the updated value): [video](/attachments/12f633af-0def-43b1-95a4-ec9a9f907dc1) (in the video, I disabled the chat re-refetch on a new message so that we always display the unread count received from `pleroma:chat_update`: the left user sends "1", the receiver gets it but the unread count is still 0, then the WS connection drops on the second message and we restore the counter on reconnect). This is a bit strange, since the unread count is recalculated in the chat view. It looks like the db query gets a different unread count when streaming out the `pleroma:chat_update` event, that doesn't include the newly created message. I suspect this might be related to the fact that [`Pipeline.common_pipeline/2`](https://git.pleroma.social/pleroma/pleroma/-/blob/6e103a18af6cfd7f454a911e2f0e1ae35cd45aa4/lib/pleroma/web/activity_pub/pipeline.ex#L19) runs in a transaction, and then inside the transaction we are spawning an async process in [`Stream.do_stream`](https://git.pleroma.social/pleroma/pleroma/-/blob/6e103a18af6cfd7f454a911e2f0e1ae35cd45aa4/lib/pleroma/web/streamer/streamer.ex#L96), and onside the DB transaction, the db query calculates a different unread count. I tried to verify this locally by removing the transaction wrapper in the `Pipeline.common_pipeline` call and it fixed the issue for me. By the way, I updated this branch so that it uses the unread value received from `pleroma:chat_update` (instead of re-fetching the chat on a new message), so the issue should be more visible now.
web-sockets1.mp4
2.8 MiB
lambadalambda commented 2020-06-04 13:53:49 +00:00
Owner
Copy link

ah, that makes sense. mhhh i'll have to see what the best way to fix this is.

ah, that makes sense. mhhh i'll have to see what the best way to fix this is.
eugenijm commented 2020-06-05 11:27:51 +00:00
Author
Member
Copy link

Updated the BE branch, it works now!

Not sure how much of an issue this is but I've noticed that the connection loss still happens sometimes during the pleroma:chat_mention notification render, and it probably has the similar cause: that is, the connection drops when the notification stream out happens before the transaction is commited and the chat_message_reference becomes avaialbe (in the NotificationView, which is called inside the streamer, we're getting the chat message from the DB query git.pleroma.social/pleroma/pleroma/-@f3ea6ee2c8/lib/pleroma/web/mastodon_api/views/notification_view.ex (L142)).

The error in the log shows the chat message reference is nil and therefore it couldn't find the view template to match this pattern

[error] Ranch listener Pleroma.Web.Endpoint.HTTP had connection process started with :cowboy_clear:start_link/4 at #PID<0.7385.0> exit with reason: 
  {%Phoenix.Template.UndefinedError{assigns: %{chat_message_reference: nil ...
{Phoenix.Template, :raise_template_not_found, 3, [file: 'lib/phoenix/template.ex', line: 341]}, 
{Pleroma.Web.MastodonAPI.NotificationView, :put_chat_message, 4, [file: 'lib/pleroma/web/mastodon_api/views/notification_view.ex', line: 144]},
{Pleroma.Web.StreamerView, :render, 3, [file: 'lib/pleroma/web/views/streamer_view.ex', line: 32]},
{Pleroma.Web.MastodonAPI.WebsocketHandler, :websocket_info, 2, [file: 'lib/pleroma/web/mastodon_api/websocket_handler.ex', line:

Made a video to illustrate this: video

Updated the BE branch, it works now! Not sure how much of an issue this is but I've noticed that the connection loss still happens sometimes during the `pleroma:chat_mention` notification render, and it probably has the similar cause: that is, the connection drops when the notification stream out happens before the transaction is commited and the chat_message_reference becomes avaialbe (in the NotificationView, which is called inside the streamer, we're getting the chat message from the DB query https://git.pleroma.social/pleroma/pleroma/-/blob/f3ea6ee2c82b2d63991d3e658566298c722ac0af/lib/pleroma/web/mastodon_api/views/notification_view.ex#L142). The error in the log shows the chat message reference is nil and therefore it couldn't find the view template to match this pattern ``` [error] Ranch listener Pleroma.Web.Endpoint.HTTP had connection process started with :cowboy_clear:start_link/4 at #PID<0.7385.0> exit with reason: {%Phoenix.Template.UndefinedError{assigns: %{chat_message_reference: nil ... {Phoenix.Template, :raise_template_not_found, 3, [file: 'lib/phoenix/template.ex', line: 341]}, {Pleroma.Web.MastodonAPI.NotificationView, :put_chat_message, 4, [file: 'lib/pleroma/web/mastodon_api/views/notification_view.ex', line: 144]}, {Pleroma.Web.StreamerView, :render, 3, [file: 'lib/pleroma/web/views/streamer_view.ex', line: 32]}, {Pleroma.Web.MastodonAPI.WebsocketHandler, :websocket_info, 2, [file: 'lib/pleroma/web/mastodon_api/websocket_handler.ex', line: ``` Made a video to illustrate this: [video](/attachments/0d085b9e-b7e4-455f-80d6-ab0ea59a4abe)
ws-connection-loss1.mp4
5.9 MiB
lambadalambda commented 2020-06-05 11:44:26 +00:00
Owner
Copy link

ah, that's streamed out too soon, too... thanks for all the reports!

ah, that's streamed out too soon, too... thanks for all the reports!
lambadalambda commented 2020-06-05 15:30:26 +00:00
Owner
Copy link

This does not check for presence of the chat feature before displaying the options, right? That is given out in https://dontbulling.me/api/v1/instance.

This does not check for presence of the chat feature before displaying the options, right? That is given out in https://dontbulling.me/api/v1/instance.
eugenijm commented 2020-06-05 16:13:57 +00:00
Author
Member
Copy link

Added the feature check

Added the feature check
lambadalambda commented 2020-06-05 16:23:19 +00:00
Owner
Copy link

Does it also hide the 'message' button in the user card?

Does it also hide the 'message' button in the user card?
eugenijm commented 2020-06-05 16:29:01 +00:00
Author
Member
Copy link

nice catch, added this too

nice catch, added this too
eugenijm commented 2020-06-06 14:58:52 +00:00
Author
Member
Copy link

should be fixed now

should be fixed now
eugenijm commented 2020-06-06 15:31:08 +00:00
Author
Member
Copy link

ok, let's not do the idle tracking

the title bar change when unfocused is working very unreliable, more than half of the times when I leave the tab, I don't get any change in the title bar to tell me there's new messages

the title bar update should be better now

ok, let's not do the idle tracking > the title bar change when unfocused is working very unreliable, more than half of the times when I leave the tab, I don't get any change in the title bar to tell me there's new messages the title bar update should be better now
lambadalambda commented 2020-06-07 06:59:59 +00:00
Owner
Copy link

Tests are failing

Tests are failing
eugenijm commented 2020-06-07 12:51:47 +00:00
Author
Member
Copy link

fixed

fixed
lambadalambda commented 2020-06-07 13:58:05 +00:00
Owner
Copy link

For some reason, the "..." icon for deleting stuff became the download icon.

For some reason, the "..." icon for deleting stuff became the download icon.
eugenijm commented 2020-06-07 14:23:07 +00:00
Author
Member
Copy link

whoops, fixed

whoops, fixed
lambadalambda commented 2020-06-07 14:33:45 +00:00
Owner
Copy link

Screenshot_2020-06-07_dontbulling_me

Image-only posts have useless empty space at the top.

![Screenshot_2020-06-07_dontbulling_me](/attachments/e3630f2e-07e9-421e-9595-75712ba10ec1) Image-only posts have useless empty space at the top.
Screenshot_2020-06-07_dontbulling_me.png
371 KiB
eugenijm commented 2020-06-07 15:01:13 +00:00
Author
Member
Copy link

fixed

fixed
lambadalambda commented 2020-06-08 07:50:26 +00:00
Owner
Copy link

one more thing, there are no 'sensitive' images here, can you remove the checkbox?

one more thing, there are no 'sensitive' images here, can you remove the checkbox?
shpuld commented 2020-06-08 08:05:04 +00:00
Owner
Copy link

Very minor issue: the images don't fully respect the cropping setting: for tall images it works like it should, but for wide images it always leaves empty space on bottom and top instead of covering the entire area even if you've allowed it to crop them.

Very minor issue: the images don't fully respect the cropping setting: for tall images it works like it should, but for wide images it always leaves empty space on bottom and top instead of covering the entire area even if you've allowed it to crop them.
lambadalambda commented 2020-06-08 08:47:35 +00:00
Owner
Copy link

Also, the 'don't mark as read when tab is not focused' thing still doesn't work correctly. it counts up for a little while but then drops to zero again.

simplescreenrecorder-2020-06-08_10.45.55

Also, the 'don't mark as read when tab is not focused' thing still doesn't work correctly. it counts up for a little while but then drops to zero again. ![simplescreenrecorder-2020-06-08_10.45.55](/attachments/88536013-5456-4a49-8da1-336732f1a182)
simplescreenrecorder-2020-06-08_10.45.55.mp4
67 KiB
eugenijm commented 2020-06-08 13:00:20 +00:00
Author
Member
Copy link

done

done
eugenijm commented 2020-06-08 13:11:45 +00:00
Author
Member
Copy link

should be fixed now (tested on firefox, chrome, safari)

should be fixed now (tested on firefox, chrome, safari)
eugenijm commented 2020-06-08 13:32:16 +00:00
Author
Member
Copy link

Looks like it works the same way with the TL layout on the latest develop (the "Don't crop the attachment in thumbnails" is unchecked) and we're reusing it for the chat layout.

Also, there is an issue with adjusting the image height dynamically: since the chat needs to be kept scrolled to the bottom, not knowing the dimensions of the image ahead of time might lead to the layout "jumping" as the image loads.

One way to fix this would be to provide the image dimensions along with the attachment in the BE API. That way, the FE could use the fixed image height and avoid the layout jumping as the image loads.

Looks like it works the same way with the TL layout on the latest develop (the "Don't crop the attachment in thumbnails" is unchecked) and we're reusing it for the chat layout. Also, there is an issue with adjusting the image height dynamically: since the chat needs to be kept scrolled to the bottom, not knowing the dimensions of the image ahead of time might lead to the layout "jumping" as the image loads. One way to fix this would be to provide the image dimensions along with the attachment in the BE API. That way, the FE could use the fixed image height and avoid the layout jumping as the image loads.
hj commented 2020-06-08 14:52:02 +00:00
Owner
Copy link

why no sensitive images support tho? seems useful even in direct conversations

why no sensitive images support tho? seems useful even in direct conversations
lambadalambda commented 2020-06-08 16:23:48 +00:00
Owner
Copy link

might come later, but for now it's not in the current backend.

might come later, but for now it's not in the current backend.
lambadalambda commented 2020-06-10 09:34:38 +00:00
Owner
Copy link

Functionality-wise, this seems to be pretty much done and works well. Could use a final code review before it goes in.

Functionality-wise, this seems to be pretty much done and works well. Could use a final code review before it goes in.
feld commented 2020-06-10 17:41:09 +00:00
Owner
Copy link

Chats have a followers-only warning that should not exist

image

Chats have a followers-only warning that should not exist ![image](/attachments/a84a486a-d17e-4b77-9e7a-517ede3daaa0)
image.png
78 KiB
eugenijm commented 2020-06-10 17:55:00 +00:00
Author
Member
Copy link

fixed

fixed
shpuld commented 2020-06-10 21:25:26 +00:00
Owner
Copy link

Huh you're right, maybe it was broken recently in develop. yeah we don't want dynamic sizing on TL nor in chats for the jumping effect, I'm too familiar with this.

Huh you're right, maybe it was broken recently in develop. yeah we don't want dynamic sizing on TL nor in chats for the jumping effect, I'm too familiar with this.
shpuld commented 2020-06-15 14:42:04 +00:00
Owner
Copy link

should be in a named class

should be in a named class
shpuld commented 2020-06-15 14:42:37 +00:00
Owner
Copy link

I don't understand this

I don't understand this
shpuld commented 2020-06-15 14:46:53 +00:00
Owner
Copy link

needs class and should prefer flex to float

needs class and should prefer flex to float
hj commented 2020-06-15 14:47:47 +00:00
Owner
Copy link

i don't entirely understand why it's exclusive to chats...

i don't entirely understand why it's exclusive to chats...
shpuld commented 2020-06-15 14:50:03 +00:00
Owner
Copy link

this seems to be for the ellipsis-button? the name should reflect that I think

this seems to be for the ellipsis-button? the name should reflect that I think
hj commented 2020-06-15 14:50:17 +00:00
Owner
Copy link

so we are removing old chats completely?

so we are removing old chats completely?
hj commented 2020-06-15 14:52:05 +00:00
Owner
Copy link

i don't think store.state would ever be empty here, neither does this code run more than once?

i don't think `store.state` would ever be empty here, neither does this code run more than once?
shpuld commented 2020-06-15 14:53:42 +00:00
Owner
Copy link

seems awfully precise, but I don't see this class being used anywhere

seems awfully precise, but I don't see this class being used anywhere
hj commented 2020-06-15 14:59:30 +00:00
Owner
Copy link

can we avoid all of this and just use plain CSS instead?

i.e. set/unset classnames?

can we avoid all of this and just use plain CSS instead? i.e. set/unset classnames?
hj commented 2020-06-15 15:00:56 +00:00
Owner
Copy link

why not const

why not const
shpuld commented 2020-06-15 15:01:23 +00:00
Owner
Copy link

there's lots of very arbitrary looking pixel values all around this file, I think it would be better to stick to certain round em values. I always get worried when I see mix of 3px, 4px, 5px, 7px for different things

there's lots of very arbitrary looking pixel values all around this file, I think it would be better to stick to certain round em values. I always get worried when I see mix of 3px, 4px, 5px, 7px for different things
hj commented 2020-06-15 15:02:58 +00:00
Owner
Copy link

if you're using global store it probably makes sense to call it "setPageFocused" and do it elsewhere?

if you're using global store it probably makes sense to call it "setPageFocused" and do it elsewhere?
hj commented 2020-06-15 15:14:20 +00:00
Owner
Copy link

a lot of redundancy here..

a lot of redundancy here..
hj commented 2020-06-15 15:14:57 +00:00
Owner
Copy link

confusing

confusing
hj commented 2020-06-15 15:15:29 +00:00
Owner
Copy link

what does this do, exactly? Why document.hidden instead of that state you stored earlier?

what does this do, exactly? Why `document.hidden` instead of that state you stored earlier?
hj commented 2020-06-15 15:16:39 +00:00
Owner
Copy link

why do you need to do something in JS on hover? what is a sequence?

why do you need to do something in JS on hover? what is a sequence?
hj commented 2020-06-15 15:17:09 +00:00
Owner
Copy link

nit: posted? not send?

nit: posted? not send?
hj commented 2020-06-15 15:19:30 +00:00
Owner
Copy link

this entire section is confusing, what is going on? could naming be even more confusing?

this entire section is confusing, what is going on? could naming be even more confusing?
shpuld commented 2020-06-15 15:20:06 +00:00
Owner
Copy link

this class seems to not be used anymore

this class seems to not be used anymore
shpuld commented 2020-06-15 15:20:34 +00:00
Owner
Copy link

could move these to the class

could move these to the class
hj commented 2020-06-15 15:22:34 +00:00
Owner
Copy link

avoid single-character variables?

avoid single-character variables?
hj commented 2020-06-15 15:24:19 +00:00
Owner
Copy link

avoid style manipulation?

avoid style manipulation?
hj commented 2020-06-15 15:25:48 +00:00
Owner
Copy link

these two blocks of code do exactly the same thing, only difference is in which element they use

these two blocks of code do exactly the same thing, only difference is in which element they use
shpuld commented 2020-06-15 15:26:33 +00:00
Owner
Copy link

any advantage for using a 'Set' over just a const array with the .includes method?

any advantage for using a 'Set' over just a const array with the .includes method?
hj commented 2020-06-15 15:28:21 +00:00
Owner
Copy link

can we keep magic numbers in constants?

can we keep magic numbers in constants?
hj commented 2020-06-15 15:37:09 +00:00
Owner
Copy link

quite a lot of unnecessary lets, some initialized some not

quite a lot of unnecessary `let`s, some initialized some not
hj commented 2020-06-15 15:39:33 +00:00
Owner
Copy link

why timeout, yet alone so big?

why timeout, yet alone so big?
hj commented 2020-06-15 15:42:15 +00:00
Owner
Copy link

???????

???????
hj commented 2020-06-15 15:45:28 +00:00
Owner
Copy link

unnecessary

unnecessary
hj commented 2020-06-15 15:46:04 +00:00
Owner
Copy link

please try to keep indentation/nesting level to minimum

please try to keep indentation/nesting level to minimum
hj commented 2020-06-15 15:47:12 +00:00
Owner
Copy link

shouldn't webpack/whatever we use autoprefix all that? what about -moz?

shouldn't webpack/whatever we use autoprefix all that? what about `-moz`?
shpuld commented 2020-06-15 15:48:21 +00:00
Owner
Copy link

is there a reason why this stuff is in 'users' module? question applies to the rest of the chat stuff in the file

is there a reason why this stuff is in 'users' module? question applies to the rest of the chat stuff in the file
hj commented 2020-06-15 15:50:05 +00:00
Owner
Copy link

you don't need to use @media outside the main selector, you can do it like so:

.chat {
  @media all { 
    rule: value
  }
}
you don't need to use `@media` outside the main selector, you can do it like so: ```scss .chat { @media all { rule: value } } ```
hj commented 2020-06-15 15:50:13 +00:00
Owner
Copy link

at least i think it does

at least i think it does
hj commented 2020-06-15 15:50:55 +00:00
Owner
Copy link

why form tag selector

why `form` tag selector
hj commented 2020-06-15 15:51:05 +00:00
Owner
Copy link

again, with nesting levels

again, with nesting levels
hj commented 2020-06-15 15:51:36 +00:00
Owner
Copy link

selector outside .chat-view

selector outside `.chat-view`
hj commented 2020-06-15 15:52:28 +00:00
Owner
Copy link

didn't you define one in App.scss?

didn't you define one in App.scss?
hj commented 2020-06-15 15:53:59 +00:00
Owner
Copy link

documentation??

documentation??
hj commented 2020-06-15 15:57:16 +00:00
Owner
Copy link

wait, we already have UserAvatar

wait, we already have UserAvatar
shpuld commented 2020-06-15 15:58:14 +00:00
Owner
Copy link

could use some tests for the file

could use some tests for the file
hj commented 2020-06-15 15:59:57 +00:00
Owner
Copy link

please include root element class as a root selector

please include root element class as a root selector
hj commented 2020-06-15 16:02:14 +00:00
Owner
Copy link

so is it 1 or 0?

so is it 1 or 0?
hj commented 2020-06-15 16:02:46 +00:00
Owner
Copy link

you mean .emoji?

you mean `.emoji`?
hj commented 2020-06-15 16:03:28 +00:00
Owner
Copy link

shouldn't all this stuff be handled by status-content?

shouldn't all this stuff be handled by `status-content`?
hj commented 2020-06-15 16:09:47 +00:00
Owner
Copy link

hypothetically, set is O(1), while array is O(n) for searching. realistically it doesn't matter at small amounts.

still, .has() is shorter than .includes()

hypothetically, set is O(1), while array is O(n) for searching. realistically it doesn't matter at small amounts. still, `.has()` is shorter than `.includes()`
hj commented 2020-06-15 16:10:25 +00:00
Owner
Copy link

is this truly necessary? maybe update popover instead?

is this truly necessary? maybe update popover instead?
hj commented 2020-06-15 16:12:04 +00:00
Owner
Copy link

i wonder how well this works with DST...

i wonder how well this works with DST...
hj commented 2020-06-15 16:14:28 +00:00
Owner
Copy link

i doubt this will work with ja_easy is this necessary?

i doubt this will work with `ja_easy` is this necessary?
hj commented 2020-06-15 16:16:36 +00:00
Owner
Copy link

we are in new chat already, aren't we?

we are in new chat already, aren't we?
hj commented 2020-06-15 16:17:57 +00:00
Owner
Copy link

title has html, html title has... screen_name?

title has html, html title has... screen_name?
hj commented 2020-06-15 16:19:49 +00:00
Owner
Copy link

i think a lot of these if branches could be rearranged in a more readable way

i think a lot of these if branches could be rearranged in a more readable way
hj commented 2020-06-15 16:20:11 +00:00
Owner
Copy link

why does both of this and the above apply for "top"?

why does both of this and the above apply for "top"?
hj commented 2020-06-15 16:21:09 +00:00
Owner
Copy link

todo what?

todo what?
hj commented 2020-06-15 16:21:32 +00:00
Owner
Copy link

image

![image](/attachments/639b72b8-2739-44a5-b8be-4193f8ccc2d8)
image.png
343 KiB
hj commented 2020-06-15 16:22:58 +00:00
Owner
Copy link

so read button doesn't reset the chat counter or...?

so read button doesn't reset the chat counter or...?
hj commented 2020-06-15 16:24:36 +00:00
Owner
Copy link

why "poster"?

why "poster"?
hj commented 2020-06-15 16:25:14 +00:00
Owner
Copy link

how many resizes you need?

how many resizes you need?
hj commented 2020-06-15 16:26:59 +00:00
Owner
Copy link

pls use click instead of mousedown/touchstart

pls use click instead of mousedown/touchstart
hj commented 2020-06-15 16:29:12 +00:00
Owner
Copy link

wouldn't it be better to just have header "Incoming" and there standard text, link, border? Look how buttons are done

wouldn't it be better to just have header "Incoming" and there standard text, link, border? Look how buttons are done
hj commented 2020-06-15 16:30:53 +00:00
Owner
Copy link

sounds like a perfect case to use a .reduce() instead

sounds like a perfect case to use a `.reduce()` instead
hj commented 2020-06-15 16:37:19 +00:00
Owner
Copy link

add chatBg layer into LAYERS with value 'underlay',
likewise add chatMessage layer with value 'chatBg'

use them here

add `chatBg` layer into `LAYERS` with value `'underlay'`, likewise add `chatMessage` layer with value `'chatBg'` use them here
hj commented 2020-06-15 16:38:13 +00:00
Owner
Copy link

why?

why?
hj commented 2020-06-15 16:38:45 +00:00
Owner
Copy link

use variable maybe?

use variable maybe?
eugenijm commented 2020-06-15 16:48:14 +00:00
Author
Member
Copy link

This is to make the mobile on-screen keyboard stay at the same position instead of disappearing after a message send (click makes it disappear). Native mobile chat apps work the way, so many users most likely expect this behavior. Useful for quickly sending a chain of short messages. Should we keep it that way or change it to a click? (Sorry, should have documented this part)

This is to make the mobile on-screen keyboard stay at the same position instead of disappearing after a message send (click makes it disappear). Native mobile chat apps work the way, so many users most likely expect this behavior. Useful for quickly sending a chain of short messages. Should we keep it that way or change it to a click? (Sorry, should have documented this part)
hj commented 2020-06-15 17:41:55 +00:00
Owner
Copy link

dunno about touchstart but mousedown should definitely change into "click", i expect buttons to react when mouse hasbeen pressed and depressed, this way you can prevent accidental misclicks, too.

dunno about touchstart but mousedown should definitely change into "click", i expect buttons to react when mouse hasbeen pressed and depressed, this way you can prevent accidental misclicks, too.
eugenijm commented 2020-06-15 21:26:43 +00:00
Author
Member
Copy link

it was empty when running the router test, just added the mocked state to the test instead

it was empty when running the router test, just added the mocked state to the test instead
eugenijm commented 2020-06-15 21:39:42 +00:00
Author
Member
Copy link

double checked that it's not necessary, removing

double checked that it's not necessary, removing
eugenijm commented 2020-06-15 21:49:55 +00:00
Author
Member
Copy link

good point, removed it

good point, removed it
eugenijm commented 2020-06-15 23:15:08 +00:00
Author
Member
Copy link

not necessary, removing that

not necessary, removing that
eugenijm commented 2020-06-16 04:05:02 +00:00
Author
Member
Copy link

The read button in the notification panel doesn't mark all the chats as read. Should it? Wasn't sure about that since BE doesn't mark them as read either.

The read button in the notification panel doesn't mark all the chats as read. Should it? Wasn't sure about that since BE doesn't mark them as read either.
eugenijm commented 2020-06-16 04:20:47 +00:00
Author
Member
Copy link

removed redundant calls

removed redundant calls
shpuld commented 2020-06-16 05:59:33 +00:00
Owner
Copy link

as long as it works as well as before in chats (really nice functionality on my phone)

I'll need to double check if regular status posting has changed much outside of chat

as long as it works as well as before in chats (really nice functionality on my phone) I'll need to double check if regular status posting has changed much outside of chat
shpuld commented 2020-06-16 07:18:50 +00:00
Owner
Copy link

image

it looks like the 'last_message' is trying to use faint text color (as evidenced by the hashtag being darker) but the rest is still normal looking

![image](/attachments/5b9d483e-ec1b-4e26-80fb-6575f27b524f) it looks like the 'last_message' is trying to use faint text color (as evidenced by the hashtag being darker) but the rest is still normal looking
image.png
13 KiB
hj commented 2020-06-16 12:45:08 +00:00
Owner
Copy link

could be a rendering bug, firefox sometimes... flickers or uses incorrect opacity for text for whatever reason, at least i've seen it on nightly quite often.

could be a rendering bug, firefox sometimes... flickers or uses incorrect opacity for text for whatever reason, at least i've seen it on nightly quite often.
hj commented 2020-06-16 12:47:22 +00:00
Owner
Copy link

when i hit "read" in notifications i expect all notifications to be marked as read, it's pretty simple, no?

when i hit "read" in notifications i expect all notifications to be marked as read, it's pretty simple, no?
shpuld commented 2020-06-16 14:10:36 +00:00
Owner
Copy link

I'll move the changes from popover to separate MR where I'll use them to fix something else, less code to review here then

I'll move the changes from popover to separate MR where I'll use them to fix something else, less code to review here then
shpuld commented 2020-06-16 15:02:39 +00:00
Owner
Copy link

Regular status posting is wonky in this branch, it posts and immediately pops up the error that you can't post an empty status with no files

Regular status posting is wonky in this branch, it posts and immediately pops up the error that you can't post an empty status with no files
feld commented 2020-06-16 15:15:33 +00:00
Owner
Copy link

ahhh! I thought this was a problem on my end the whole time.

ahhh! I thought this was a problem on my end the whole time.
hj commented 2020-06-16 15:18:35 +00:00
Owner
Copy link

most likely related to mousedown thing...

most likely related to mousedown thing...
eugenijm commented 2020-06-16 20:16:58 +00:00
Author
Member
Copy link

the double send issue should be fixed now

the double send issue should be fixed now
eugenijm commented 2020-06-16 22:24:58 +00:00
Author
Member
Copy link

added the docs to describe the scroll positioning

added the docs to describe the scroll positioning
eugenijm commented 2020-06-16 22:30:13 +00:00
Author
Member
Copy link

Just to clarify, chat messages are not displayed in the notifications panel and are not included in the notification panel unread counter, hence two different counters (the unread chat count displays the data from GET /api/v1/pleroma/chats)

Just to clarify, chat messages are not displayed in the notifications panel and are not included in the notification panel unread counter, hence two different counters (the unread chat count displays the data from `GET /api/v1/pleroma/chats`)
eugenijm commented 2020-06-16 22:37:26 +00:00
Author
Member
Copy link

removing the focus state, not necessary for the chat

removing the focus state, not necessary for the chat
eugenijm commented 2020-06-16 22:42:44 +00:00
Author
Member
Copy link

returned that old chats

returned that old chats
eugenijm commented 2020-06-16 22:54:29 +00:00
Author
Member
Copy link

confirmed it adds the autoprefix

confirmed it adds the autoprefix
eugenijm commented 2020-06-16 23:05:18 +00:00
Author
Member
Copy link

refactored this part

refactored this part
eugenijm commented 2020-06-16 23:05:45 +00:00
Author
Member
Copy link

The reason for setting the fixed height is the mobile browser panel can sometimes hide or overlap the posting form. I tried different approaches here but setting the fixed height appeared to be the most reliable one

The reason for setting the fixed height is the mobile browser panel can sometimes hide or overlap the posting form. I tried different approaches here but setting the fixed height appeared to be the most reliable one
shpuld commented 2020-06-17 04:48:54 +00:00
Owner
Copy link

right, so an object would work the same way

right, so an object would work the same way
shpuld commented 2020-06-17 05:21:38 +00:00
Owner
Copy link

yeah this is a misunderstanding, the "read" should not mark chats as read, as you don't see chat notifications in the notification column, they are separate. the way it already works in practice is really nice 👍

yeah this is a misunderstanding, the "read" should not mark chats as read, as you don't see chat notifications in the notification column, they are separate. the way it already works in practice is really nice :thumbsup:
shpuld commented 2020-06-17 05:38:43 +00:00
Owner
Copy link

I investigated, the problem seems to be using "--faintText" instead of "--faint"

I investigated, the problem seems to be using "--faintText" instead of "--faint"
eugenijm commented 2020-06-17 10:48:14 +00:00
Author
Member
Copy link

thanks, changed it to "--faint"

thanks, changed it to "--faint"
eugenijm commented 2020-06-17 11:00:41 +00:00
Author
Member
Copy link

good point, added the incoming and outgoing headers

good point, added the incoming and outgoing headers
eugenijm commented 2020-06-17 12:22:52 +00:00
Author
Member
Copy link

ah, that was unnecessary, removed it

ah, that was unnecessary, removed it
eugenijm commented 2020-06-17 12:24:14 +00:00
Author
Member
Copy link

refactored this part

refactored this part
eugenijm commented 2020-06-17 12:25:30 +00:00
Author
Member
Copy link

thanks, confirmed it works inside the selector

thanks, confirmed it works inside the selector
eugenijm commented 2020-06-17 14:21:48 +00:00
Author
Member
Copy link

Renamed sequence to messageChain for better clarity, it's an uninterrupted chain of messages made by the same user.

A message chain has one user avatar, aligned with the first message of the chain. When "play-on-hover GIFs" is enabled, we want to animate the avatar when the chain is hovered, so JS is used to pass this state to the first message in the chain.

Renamed sequence to messageChain for better clarity, it's an uninterrupted chain of messages made by the same user. A message chain has one user avatar, aligned with the first message of the chain. When "play-on-hover GIFs" is enabled, we want to animate the avatar when the chain is hovered, so JS is used to pass this state to the first message in the chain.
hj commented 2020-06-17 15:34:44 +00:00
Owner
Copy link

IIRC StillImage relies purely on CSS to do hover, why do you need JS here then?

IIRC StillImage relies purely on CSS to do hover, why do you need JS here then?
eugenijm commented 2020-06-17 16:25:08 +00:00
Author
Member
Copy link

For this effect: video

The avatar near the first message of the chain is animated when any message belonging to the chain is hovered.

AFAIK, we could do this with CSS if the chain was wrapped in a parent element, but then it would have bring more complexity to the layout.

For this effect: [video](/attachments/c875df08-d19d-48b9-9aa4-9926bd72f710) The avatar near the first message of the chain is animated when any message belonging to the chain is hovered. AFAIK, we could do this with CSS if the chain was wrapped in a parent element, but then it would have bring more complexity to the layout.
animate-on-chain-hover.mp4
209 KiB
eugenijm commented 2020-06-17 17:08:28 +00:00
Author
Member
Copy link

renamed to sendMessage

renamed to sendMessage
eugenijm commented 2020-06-17 20:30:25 +00:00
Author
Member
Copy link

refactored it so that const is preferred when no rebinding is happening

refactored it so that const is preferred when no rebinding is happening
eugenijm commented 2020-06-17 20:49:23 +00:00
Author
Member
Copy link

good point, renamed to send

good point, renamed to send
hj commented 2020-06-17 21:07:19 +00:00
Owner
Copy link

could you please leave a comment about that in the code? Not sure about usability but we'll have to wait and see

could you please leave a comment about that in the code? Not sure about usability but we'll have to wait and see
eugenijm commented 2020-06-17 21:53:52 +00:00
Author
Member
Copy link

sure, left the comment

sure, left the comment
eugenijm commented 2020-06-18 04:33:45 +00:00
Author
Member
Copy link

done

done
eugenijm commented 2020-06-18 08:21:50 +00:00
Author
Member
Copy link

refactored this part

refactored this part
eugenijm commented 2020-06-18 08:39:07 +00:00
Author
Member
Copy link

Updated this, should be more readable now

we position the panel or picker above the input when placement is set to top (the chat form) or
when placement is set to auto (default) and the element doesn't have enough space at the bottom.

Updated this, should be more readable now we position the panel or picker above the input when `placement` is set to `top` (the chat form) or when `placement` is set to `auto` (default) and the element doesn't have enough space at the bottom.
eugenijm commented 2020-06-18 09:00:35 +00:00
Author
Member
Copy link

updated it to use more descriptive naming and added a comment

updated it to use more descriptive naming and added a comment
eugenijm commented 2020-06-18 09:53:00 +00:00
Author
Member
Copy link

unnecessary, removing that

unnecessary, removing that
eugenijm commented 2020-06-18 09:54:20 +00:00
Author
Member
Copy link

refactored this, not necessary anymore

refactored this, not necessary anymore
eugenijm commented 2020-06-18 10:05:56 +00:00
Author
Member
Copy link

thanks, updated the branch!

thanks, updated the branch!
eugenijm commented 2020-06-18 11:31:19 +00:00
Author
Member
Copy link

oh, right, removing that

oh, right, removing that
eugenijm commented 2020-06-18 19:28:20 +00:00
Author
Member
Copy link

Chat navigation feels nicer w/o fade but I wasn't sure about modifying the existing behavior for other routes

Chat navigation feels nicer w/o fade but I wasn't sure about modifying the existing behavior for other routes
eugenijm commented 2020-06-18 19:29:02 +00:00
Author
Member
Copy link

removed the additional state, document.hidden seem to be enough for this use case

removed the additional state, `document.hidden` seem to be enough for this use case
eugenijm commented 2020-06-18 19:30:03 +00:00
Author
Member
Copy link

ah, right, this isn't very clear in this context, maybe request?

ah, right, this isn't very clear in this context, maybe `request`?
eugenijm commented 2020-06-18 21:43:51 +00:00
Author
Member
Copy link

you're right, removed that in favor of status-content

you're right, removed that in favor of `status-content`
eugenijm commented 2020-06-18 21:45:20 +00:00
Author
Member
Copy link

replaced by status-content too

replaced by `status-content` too
eugenijm commented 2020-06-18 21:47:42 +00:00
Author
Member
Copy link

reduced the nesting in scss

reduced the nesting in scss
eugenijm commented 2020-06-18 23:51:19 +00:00
Author
Member
Copy link

Changed fg to border in chatMessageIncomingBorder, seem to be working fine with all the themes. Removed the extra colors in the redmond themes as they are not necessary anymore.

Changed `fg` to `border` in `chatMessageIncomingBorder`, seem to be working fine with all the themes. Removed the extra colors in the redmond themes as they are not necessary anymore.
eugenijm commented 2020-06-19 06:07:14 +00:00
Author
Member
Copy link

changed it to flex

changed it to flex
eugenijm commented 2020-06-19 09:13:53 +00:00
Author
Member
Copy link

Refactored the counters style so that they reuse the existing badge and badge-notifications. There are a few small differences between the new message counter that is displayed inside the "jump-to-the-bottom" button and the chat counter in the nav panel, so I created the corresponding .unread-message-count and .unread-chat-count.

Refactored the counters style so that they reuse the existing `badge` and `badge-notifications`. There are a few small differences between the new message counter that is displayed inside the "jump-to-the-bottom" button and the chat counter in the nav panel, so I created the corresponding `.unread-message-count` and `.unread-chat-count`.
hj commented 2020-06-19 17:45:40 +00:00
Owner
Copy link

i'm not sure what poster is supposed to be in the beginning, is it like postHandler maybe?

i'm not sure what poster is supposed to be in the beginning, is it like `postHandler` maybe?
hj commented 2020-06-19 18:07:01 +00:00
Owner
Copy link

tried running locally with updated backend (recent develop), didn't work all that well

image

backend log

Jun 19 20:05:00 shigusegubu3 mix[18115]: 20:05:00.985 request_id=FhoEE7gkInQivvwAA1Gk [error] Internal server error: %ArgumentError{message: "argument error"}
Jun 19 20:05:00 shigusegubu3 mix[18115]: 20:05:00.986 request_id=FhoEE7gkInQivvwAA1Gk [info] Converted error :badarg to 500 response

FE managed to create a new chat but trying to send messages freezes the submit form - there's no error handling.

tried running locally with updated backend (recent develop), didn't work all that well ![image](/attachments/fc9b094d-d10d-4cf5-8708-dfc9af62588d) backend log ``` Jun 19 20:05:00 shigusegubu3 mix[18115]: 20:05:00.985 request_id=FhoEE7gkInQivvwAA1Gk [error] Internal server error: %ArgumentError{message: "argument error"} Jun 19 20:05:00 shigusegubu3 mix[18115]: 20:05:00.986 request_id=FhoEE7gkInQivvwAA1Gk [info] Converted error :badarg to 500 response ``` FE managed to create a new chat but trying to send messages freezes the submit form - there's no error handling.
image.png
45 KiB
eugenijm commented 2020-06-20 06:59:44 +00:00
Author
Member
Copy link

Added the error handling for chat creation and message sending (the recent develop works fine for me though).

Added the error handling for chat creation and message sending (the recent develop works fine for me though).
eugenijm commented 2020-06-22 06:22:32 +00:00
Author
Member
Copy link

Removed in favor of UserAvatar

Removed in favor of UserAvatar
eugenijm commented 2020-06-22 06:29:12 +00:00
Author
Member
Copy link

kind of, the function is either status_poster.postStatus or chat.sendMessage. Both work the same way: make an API request, add a message or status to the store, return the result back to the form. postHanlder sounds good

kind of, the function is either [`status_poster.postStatus`](https://git.pleroma.social/pleroma/pleroma-fe/-/blob/6e2810b8bc449c628d461aa02f70d689ac304c3a/src/services/status_poster/status_poster.service.js#L4-33) or [`chat.sendMessage`](https://git.pleroma.social/pleroma/pleroma-fe/-/blob/e006fd23d5e6c5049a7ea8e8dbac1605080a5047/src/components/chat/chat.js#L281-310). Both work the same way: make an API request, add a message or status to the store, return the result back to the form. `postHanlder` sounds good
eugenijm commented 2020-06-22 08:33:32 +00:00
Author
Member
Copy link

Replaced px by em, with the exception of a few places where they rely on other elements having px (the user avatar width, the top bar, and also border radius and shadows)

Replaced px by em, with the exception of a few places where they rely on other elements having px (the user avatar width, the top bar, and also border radius and shadows)
eugenijm commented 2020-06-22 16:04:26 +00:00
Author
Member
Copy link

This is for the mobile chat app to make the mobile top address bar and the bottom bar not to go away on scroll up / down. Is this also a desirable behavior for the mobile timeline?

ah, nvm, sure, classnames would be better

edit: done

~~This is for the mobile chat app to make the mobile top address bar and the bottom bar not to go away on scroll up / down. Is this also a desirable behavior for the mobile timeline?~~ ah, nvm, sure, classnames would be better edit: done
hj commented 2020-06-22 22:42:42 +00:00
Owner
Copy link

if you insist so much on using inconvenient Enter to send, make sure Ctrl-enter adds newline in such mode, right now it also sends

if you insist so much on using ~~inconvenient~~ Enter to send, make sure Ctrl-enter adds newline in such mode, right now it **also** sends
hj commented 2020-06-22 22:47:14 +00:00
Owner
Copy link

panel shadow isn't applied to chat panel

panel shadow isn't applied to chat panel
hj commented 2020-06-22 22:48:44 +00:00
Owner
Copy link

upload button being disabled isn't visible, only cursor gives away that it's disabled.

upload button being disabled isn't visible, only cursor gives away that it's disabled.
eugenijm commented 2020-06-23 19:18:36 +00:00
Author
Member
Copy link

added the tests for chat service

added the tests for chat service
eugenijm commented 2020-06-23 19:18:56 +00:00
Author
Member
Copy link

fixed

fixed
eugenijm commented 2020-06-24 01:39:11 +00:00
Author
Member
Copy link

removed unused state

removed unused state
eugenijm commented 2020-06-24 03:20:33 +00:00
Author
Member
Copy link

fixed

fixed
eugenijm commented 2020-06-24 10:11:40 +00:00
Author
Member
Copy link

ok, newline on ctrl+enter seems to be used in other chats too (whatsapp, telegram, discord, slack, twitter DMs), so I guess it's also an expected behavior the same way send on enter is. cc @shpuld (re: #2286)

ok, newline on ctrl+enter seems to be used in other chats too (whatsapp, telegram, discord, slack, twitter DMs), so I guess it's also an expected behavior the same way send on enter is. cc @shpuld (re: https://git.pleroma.social/pleroma/pleroma-fe/pulls/2286#note_61194)
lambadalambda commented 2020-06-24 11:48:21 +00:00
Owner
Copy link

can you make both ctrl+enter and shift+enter do a newline? I see that other chats also do that.

can you make both ctrl+enter and shift+enter do a newline? I see that other chats also do that.
eugenijm commented 2020-06-24 11:57:11 +00:00
Author
Member
Copy link

yup, added the newline on ctrl+enter behavior in the last commit

yup, added the newline on ctrl+enter behavior in the last commit
shpuld commented 2020-06-24 12:00:37 +00:00
Owner
Copy link

I thought it's weird when ctrl-enter doesn't send alongside just enter, shift-enter is the safe way to add newlines. however if it's common in other software to use ctrl-enter for newlines, then I'm not against it

I thought it's weird when ctrl-enter doesn't send alongside just enter, shift-enter is the safe way to add newlines. however if it's common in other software to use ctrl-enter for newlines, then I'm not against it
eugenijm commented 2020-06-24 13:18:30 +00:00
Author
Member
Copy link

ok, marking this as resolved

ok, marking this as resolved
eugenijm commented 2020-06-25 21:49:13 +00:00
Author
Member
Copy link

Sorry in advance if overexplaning. On the BE, the message timestamp is represented by NaiveDateTime which doesn't contain the timezone info: https://hexdocs.pm/elixir/NaiveDateTime.html from the docs

datetime may not actually exist in certain areas in the world even though it is valid

When the given message timestamp doesn't exists in the client's timezone, JS adds/subtracts an hour.

E.g., when DST starts and the clocks are turned forward, the client skips an hour

new Date("2010-03-27T22:47:47.000Z") // Sun Mar 28 2010 01:47:47 GMT+0300 (Moscow Standard Time)
new Date("2010-03-27T23:47:47.000Z") // Sun Mar 28 2010 03:47:47 GMT+0400 (Moscow Summer Time)

however, when DST ends and the clocks are turned backward, we might get an unusual artifact in the UI when the displayed time in hours and minutes of a more recent message is less than that of an older one. E.g.,

new Date("2010-10-30T22:35:00.000Z") // Sun Oct 31 2010 02:35:00 GMT+0400 (Moscow Summer Time)
new Date("2010-10-30T23:25:00.000Z") // Sun Oct 31 2010 02:25:00 GMT+0300 (Moscow Standard Time)

this doesn't affect the actual order of messages, ofc, since they are sorted by ids.

As for the date separator, it's inserted before the first message that has the date greater than the previous one. So, with respect to the message list, it's inserted as expected, e.g., in this example, it will be displayed this way:

older messages (Oct. 30) ...

--- Today (Oct 31) ---

02:35
02:25

the order and the date separator position are correct, the only issue is the overlap in the displayed hours and minutes between messages created during the "unreal" for the client hour and the messages created in the next hour.

Sorry in advance if overexplaning. On the BE, the message timestamp is represented by [`NaiveDateTime`](https://git.pleroma.social/pleroma/pleroma/-/blob/5d60b25e690eb21ad2539a10036ba39489f62f97/lib/pleroma/web/common_api/utils.ex#L384) which doesn't contain the timezone info: https://hexdocs.pm/elixir/NaiveDateTime.html from the docs > datetime may not actually exist in certain areas in the world even though it is valid When the given message timestamp doesn't exists in the client's timezone, JS adds/subtracts an hour. E.g., when DST starts and the clocks are turned forward, the client skips an hour ``` new Date("2010-03-27T22:47:47.000Z") // Sun Mar 28 2010 01:47:47 GMT+0300 (Moscow Standard Time) new Date("2010-03-27T23:47:47.000Z") // Sun Mar 28 2010 03:47:47 GMT+0400 (Moscow Summer Time) ``` however, when DST ends and the clocks are turned backward, we might get an unusual artifact in the UI when the displayed time in hours and minutes of a more recent message is less than that of an older one. E.g., ``` new Date("2010-10-30T22:35:00.000Z") // Sun Oct 31 2010 02:35:00 GMT+0400 (Moscow Summer Time) new Date("2010-10-30T23:25:00.000Z") // Sun Oct 31 2010 02:25:00 GMT+0300 (Moscow Standard Time) ``` this doesn't affect the actual order of messages, ofc, since they are sorted by ids. As for the date separator, it's inserted before the first message that has the date greater than the previous one. So, with respect to the message list, it's inserted as expected, e.g., in this example, it will be displayed this way: ``` older messages (Oct. 30) ... --- Today (Oct 31) --- 02:35 02:25 ``` the order and the date separator position are correct, the only issue is the overlap in the displayed hours and minutes between messages created during the "unreal" for the client hour and the messages created in the next hour.
hj commented 2020-06-26 00:29:09 +00:00
Owner
Copy link

thanks, seems fine then.

thanks, seems fine then.
hj commented 2020-06-26 00:37:36 +00:00
Owner
Copy link

why do you need computedStyle tho? why not offsetHeight or such?

why do you need computedStyle tho? why not offsetHeight or such?
eugenijm commented 2020-06-26 08:18:02 +00:00
Author
Member
Copy link

thanks, I haven't tried offsetHeight, seem to be working great on mobile and desktop. edit: confirmed it works on ios and android mobile browsers and doesn't cause the overlap between the posting form and browser bars

thanks, I haven't tried offsetHeight, seem to be working great on mobile and desktop. edit: confirmed it works on ios and android mobile browsers and doesn't cause the overlap between the posting form and browser bars
lambadalambda commented 2020-07-02 08:23:39 +00:00
Owner
Copy link

@hj @shpuld anything left to do here?

@hj @shpuld anything left to do here?
hj commented 2020-07-02 09:45:52 +00:00
Owner
Copy link

final round of review and maybe dogfood it a bit more. i think i'll merge it into sgsgb branch.

final round of review and maybe dogfood it a bit more. i think i'll merge it into sgsgb branch.
lambadalambda commented 2020-07-02 09:53:37 +00:00
Owner
Copy link

i've been using this branch for a month now, works well

i've been using this branch for a month now, works well
shpuld commented 2020-07-02 09:53:43 +00:00
Owner
Copy link

yeah, I've been putting off the 2k+ re-review but I'll start today

yeah, I've been putting off the 2k+ re-review but I'll start today
shpuld commented 2020-07-02 09:54:11 +00:00
Owner
Copy link

found a new minor bug with the layout:

image

found a new minor bug with the layout: ![image](/attachments/d26ea475-d85d-4bab-b8b4-d0505b27d2b6)
image.png
38 KiB
hj commented 2020-07-02 12:01:36 +00:00
Owner
Copy link

UX problem: there's currently absolutely no indication if message reached target instance or not, i.e. sending chat messages to mastodon seem to work like normal, but I imagine mastosoc never processed the message and probably errored out. Can anything be done about it? @lambadalambda

UX problem: there's currently absolutely no indication if message reached target instance or not, i.e. sending chat messages to mastodon seem to work like normal, but I imagine mastosoc never processed the message and probably errored out. Can anything be done about it? @lambadalambda
hj commented 2020-07-02 12:07:28 +00:00
Owner
Copy link

inserting newline via Ctrl-Enter doesn't scroll/resize the textbox

inserting newline via Ctrl-Enter doesn't scroll/resize the textbox
eugenijm commented 2020-07-02 13:02:35 +00:00
Author
Member
Copy link

Fixed

Fixed
shpuld commented 2020-07-02 14:42:12 +00:00
Owner
Copy link

my vote is to remove the fade for everything to be honest, it just looks tacky and slow

my vote is to remove the fade for everything to be honest, it just looks tacky and slow
lambadalambda commented 2020-07-02 15:33:08 +00:00
Owner
Copy link

adding info about that to https://git.pleroma.social/pleroma/pleroma-meta/-/issues/41

adding info about that to https://git.pleroma.social/pleroma/pleroma-meta/-/issues/41
hj commented 2020-07-02 15:56:18 +00:00
Owner
Copy link

is it possible to, say, error out when trying to send user a chat message if remote server rejects message for now?

is it possible to, say, error out when trying to send user a chat message if remote server rejects message for now?
lambadalambda commented 2020-07-02 16:06:25 +00:00
Owner
Copy link

sadly it isn't, because AP messages are always acknowledged by the receiving server as the actual processing happens asynchronously, so unknown messages are just silently ignored.

sadly it isn't, because AP messages are always acknowledged by the receiving server as the actual processing happens asynchronously, so unknown messages are just silently ignored.
eugenijm commented 2020-07-03 09:44:51 +00:00
Author
Member
Copy link

Fixed

Fixed
eugenijm commented 2020-07-04 13:28:49 +00:00
Author
Member
Copy link

ok, removed the fade on route change completely

ok, removed the fade on route change completely
shpuld commented 2020-07-07 11:26:02 +00:00
Owner
Copy link

I don't think we need to force the chat list to be full height with pixel perfect heights, I think a min height of something like 25em would work well, not too awkward on mobile or desktop even when empty. we're not afraid of showing background image :)

  min-height: 25em;
I don't think we need to force the chat list to be full height with pixel perfect heights, I think a min height of something like 25em would work well, not too awkward on mobile or desktop even when empty. we're not afraid of showing background image :) ```suggestion:-0+0 min-height: 25em; ```
shpuld commented 2020-07-07 11:30:33 +00:00
Owner
Copy link

there doesn't seem to be any placeholder when the chats list is empty? would be a good idea to add a simple faint text like

image

there doesn't seem to be any placeholder when the chats list is empty? would be a good idea to add a simple faint text like ![image](/attachments/2111bd4b-3b5f-4415-ac1f-aa8fe485859a)
image.png
166 KiB
shpuld commented 2020-07-07 12:42:18 +00:00
Owner
Copy link

this seems like a good change but I wonder if existing code is relying on the old behavior?

this seems like a good change but I wonder if existing code is relying on the old behavior?
shpuld commented 2020-07-07 12:51:19 +00:00
Owner
Copy link

really, the code is looking pretty good :) this is very close to being merged imo

really, the code is looking pretty good :) this is very close to being merged imo
hj commented 2020-07-07 13:35:18 +00:00
Owner
Copy link

avoid setting styles directly and instead use classes?

avoid setting styles directly and instead use classes?
hj commented 2020-07-07 13:36:56 +00:00
Owner
Copy link

same here

same here
hj commented 2020-07-07 13:57:45 +00:00
Owner
Copy link

again, not liking the direct styles

again, not liking the direct styles
hj commented 2020-07-07 18:16:13 +00:00
Owner
Copy link

image

low-hanging characters are cut off

![image](/attachments/32316082-3baa-43fd-aa1d-a48a8f2a15c0) low-hanging characters are cut off
image.png
7.6 KiB
hj commented 2020-07-07 18:21:12 +00:00
Owner
Copy link

apart from direct styles, code looks good

testing-wise seems ok too, apart from that line cutoff

apart from direct styles, code looks good testing-wise seems ok too, apart from that line cutoff
hj commented 2020-07-07 18:21:41 +00:00
Owner
Copy link

i would probably add a single-line modifier to status-content that uses text-overflow and prohibits multiline

i would probably add a `single-line` modifier to status-content that uses `text-overflow` and prohibits multiline
eugenijm commented 2020-07-07 18:37:36 +00:00
Author
Member
Copy link

Ah, I haven't considered it. Yeah, for extra safety it's better to keep the existing behavior and changing it is probably outside the scope of the MR since it might affect the other parts

Ah, I haven't considered it. Yeah, for extra safety it's better to keep the existing behavior and changing it is probably outside the scope of the MR since it might affect the other parts
eugenijm commented 2020-07-07 19:22:28 +00:00
Author
Member
Copy link

In this case, we're passing popoverMarginStyle to the Popover's margin prop that uses it for calculating the popover position

In this case, we're passing `popoverMarginStyle` to [the Popover's `margin` prop](https://git.pleroma.social/pleroma/pleroma-fe/-/blob/7fcd0d49c46afd15ccc9794a36505b89b87ff5e8/src/components/popover/popover.js#L17) that uses it for calculating the popover position
eugenijm commented 2020-07-07 19:22:48 +00:00
Author
Member
Copy link

Fixed

Fixed
eugenijm commented 2020-07-07 20:35:13 +00:00
Author
Member
Copy link

Added the placeholder

Added the placeholder
eugenijm commented 2020-07-08 00:52:43 +00:00
Author
Member
Copy link

Thanks for the suggestion, added it

Thanks for the suggestion, added it
shpuld commented 2020-07-08 10:06:32 +00:00
Owner
Copy link
  }

this suggestion fixes the bottom corners of the chat list being sharp when they shouldn't
image

```suggestion:-6+0 } ``` this suggestion fixes the bottom corners of the chat list being sharp when they shouldn't ![image](/attachments/0d04edcf-5d68-4b2d-a844-0f8ffd0392d4)
image.png
401 KiB
eugenijm commented 2020-07-08 21:05:59 +00:00
Author
Member
Copy link

Now that chats are no longer direct conversations, would it be a good idea to add the "Message" button back to the user profile card for quicker access instead of having it in the account menu under the ellipsis button? I've noticed that twitter has the DM button more easily accessible. Might be easier to discover for the new users

Now that chats are no longer direct conversations, would it be a good idea to add the "Message" button back to the user profile card for quicker access instead of having it in the account menu under the ellipsis button? I've noticed that twitter has the DM button more easily accessible. Might be easier to discover for the new users
lambadalambda commented 2020-07-10 08:54:16 +00:00
Owner
Copy link

Tested, tested, tested and continues to work well. Any final change requests? Otherwise i'd say it's time to hit hat button.

Tested, tested, tested and continues to work well. Any final change requests? Otherwise i'd say it's time to hit hat button.
shpuld commented 2020-07-10 09:09:56 +00:00
Owner
Copy link

congrats for sticking and finishing with such a long task @eugenijm !! and sorry for the process being so difficult

congrats for sticking and finishing with such a long task @eugenijm !! and sorry for the process being so difficult
eugenijm commented 2020-07-10 14:13:46 +00:00
Author
Member
Copy link

thanks for the all the feedback and reviews @hj @shpuld @lambadalambda!

thanks for the all the feedback and reviews @hj @shpuld @lambadalambda!

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
BE Request

   
Backlog

   
Better behavior

   
Blocked by deps

   
Bug

   
Code Review

   
Discussion

   
Doing

   
Feature Request / Enhancement

   
In the shining bright future maybe

   
It's complicated

   
MS Edge

   
Mememoon (or other niche browsers)

   
Missing API

   
Missing feature

   
Missing l10n/i18n

   
NL1

Instance Admin Dashboard

  
NL2

Improved Mobile Experience

  
NL4

Improved instance customization and QoL options

  
Need to verify on develop

   
RELEASE BLOCKER

   
Reassign or Close

   
Refactor

   
Regression

   
Safari

   
To Do

   
User story

   
Waiting on godot

   
accessibility

https://git.pleroma.social/pleroma/pleroma-fe/-/snippets/7746

  
better documentation

   
chore

   
confirmed

   
easy ticket

   
incident

Denotes a disruption to IT services and the associated issues require immediate attention

  
l10n update

   
mastoapi

   
mobile

   
needs design

   
needs-info

   
needs-review

   
stupid
No labels
BE Request
Backlog
Better behavior
Blocked by deps
Bug
Code Review
Discussion
Doing
Feature Request / Enhancement
In the shining bright future maybe
It's complicated
MS Edge
Mememoon (or other niche browsers)
Missing API
Missing feature
Missing l10n/i18n
NL1
NL2
NL4
Need to verify on develop
RELEASE BLOCKER
Reassign or Close
Refactor
Regression
Safari
To Do
User story
Waiting on godot
accessibility
better documentation
chore
confirmed
easy ticket
incident
l10n update
mastoapi
mobile
needs design
needs-info
needs-review
stupid
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
8 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
pleroma/pleroma-fe!2286
No description provided.