Added utility functions for calculating overlay position and
another one for generating actions that will end up in overlay.
Details
Details
Functions visible, but not called anywhere yet.
Will test in final diff.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| web/chat/mention-utils.js | ||
|---|---|---|
| 42–45 ↗ | (On Diff #18818) | This change should've been made in the diff that introduced this code |
| 75–82 ↗ | (On Diff #18818) | This logic is hard to follow ans can be simplified (subtracting offsetLeft from offsetLeft can be omitted) |
| 92 ↗ | (On Diff #18818) | We should either use shorthand array syntax or read only array. When returning value generated in a function, we can use mutable array. |
| 94 ↗ | (On Diff #18818) | What if there are multiple users without username? We probably should filter them out, but definitely we shouldn't have duplicated keys. User id sounds like a lot safer option to use as a key. |
| 104 ↗ | (On Diff #18818) | You can simplify it: if newSuffixText.length === 0 then newSuffixText[0] === undefined so the first condition isn't required |
| 111 ↗ | (On Diff #18818) | Why do we need this? |
Comment Actions
Responded to inlines and fixed code.
| web/chat/mention-utils.js | ||
|---|---|---|
| 42–45 ↗ | (On Diff #18818) | Right, moved those changes to: https://phab.comm.dev/D5674 |
| 75–82 ↗ | (On Diff #18818) | yeah, I felt that it was clumsy, but couldn't find a better way. That's neat, thanks |
| 92 ↗ | (On Diff #18818) | Changing to readonly |
| 94 ↗ | (On Diff #18818) | Right, fixed both. Filtering out anonymous users and using id as a key. |
| 104 ↗ | (On Diff #18818) | Ah right, forgot JS handles it that way. |
| 111 ↗ | (On Diff #18818) | You served as a rubber duck. I've spent like 15 minutes trying to write up explanation and came up with a nicer way to write it. Now space character is included in newSuffixText so we can simply subtract its length and then move 1 character to the right, so the caret is after that one space. |
Comment Actions
Had some problems with our git workflow and left that as an artifact. Sorry for that, fixed now.