In chat export, make sender name differentiable using `< >`
Fixes https://github.com/vector-im/element-web/issues/23837
Checklist
- [x] Tests written for new code (and old code if feasible)
- [x] Linter and other CI checks pass
- [x] Sign-off given on the changes (see CONTRIBUTING.md)
Signed-off-by: Ramesh Singh @rameshsinghppc
Type: [enhancement/defect/task]
Here's what your changelog entry will look like:
🐛 Bug Fixes
- In chat export, make sender name differentiable using
< >(#10748). Fixes vector-im/element-web#23837. Contributed by @rameshsinghppc.
This appears to be much the same as https://github.com/matrix-org/matrix-react-sdk/pull/10013, and as such has the same problems:
This code is used throughout the whole product and not only in the context of chat exports. You will also need to write tests for this PR to be merged as per then contributing guidelines https://github.com/vector-im/element-web/blob/develop/CONTRIBUTING.md
Please read CONTRIBUTING.md which gives instructions on how to make a good pull request. Looking at the format of other pull requests on the repository should help too.
I have sent PR(https://github.com/matrix-org/matrix-js-sdk/pull/3345#issue-1698769126), please review it
Both this and the other new PR are currently not in a state that can be reviewed or merged.
If you genuinely want to contribute this, I'd suggest adding another parameter to the functions in TextForEvent and using that to control whether the sender name will be wrapped in brackets. Take a look at the parameter that's used for controlling whether hidden events are included as an example.
@rameshsinghppc are you still keen to work on this? If you need pointers on how to fix up the CI failures then let us know.
@rameshsinghppc are you still keen to work on this? If you need pointers on how to fix up the CI failures then let us know.
@richvdh Yes, please help me to fix these failures.
@richvdh Yes, please help me to fix these failures.
Ok, what have you tried so far and which parts are confusing you?
@richvdh Yes, please help me to fix these failures.
Ok, what have you tried so far and which parts are confusing you? I don't know how to fix the eslint problem and for typescript syntax check some function are not available during pull request checks but on my local machine it's working fine.
Right; a lot of the problem here is that it is building against your fork of matrix-js-sdk (https://github.com/rameshsinghppc/matrix-js-sdk) which is outdated. (We have some magical branch matching.) You can see this happening at https://github.com/matrix-org/matrix-react-sdk/actions/runs/5159164960/jobs/9322532146#step:4:48.
Suggest, as a first step, you pull the latest develop into your fork of matrix-js-sdk. Ping me once you've done so and I'll re-run CI here.
Right; a lot of the problem here is that it is building against your fork of
matrix-js-sdk(https://github.com/rameshsinghppc/matrix-js-sdk) which is outdated. (We have some magical branch matching.) You can see this happening at https://github.com/matrix-org/matrix-react-sdk/actions/runs/5159164960/jobs/9322532146#step:4:48.Suggest, as a first step, you pull the latest
developinto your fork ofmatrix-js-sdk. Ping me once you've done so and I'll re-run CI here.
To be more accurate here: it's actually building against the branch named rameshsingh in your fork of matrix-js-sdk. (Our magic branch-matching will use a js-sdk branch with the same name as your react-sdk branch).
So you need to update that branch (or just delete it).
Right; a lot of the problem here is that it is building against your fork of
matrix-js-sdk(https://github.com/rameshsinghppc/matrix-js-sdk) which is outdated. (We have some magical branch matching.) You can see this happening at https://github.com/matrix-org/matrix-react-sdk/actions/runs/5159164960/jobs/9322532146#step:4:48. Suggest, as a first step, you pull the latestdevelopinto your fork ofmatrix-js-sdk. Ping me once you've done so and I'll re-run CI here.To be more accurate here: it's actually building against the branch named
rameshsinghin your fork ofmatrix-js-sdk. (Our magic branch-matching will use a js-sdk branch with the same name as your react-sdk branch).So you need to update that branch (or just delete it).
@richvdh Thank you for your help. I just deleted that repository.
you don't need to paste a screenshot - I can see the errors in CI.
Look at the line of code it is complaining about:
else return textForEvent(mxEv, this.room.client,true) + mediaText;
What is the return type of textForEvent? (Hint: look at the typescript definitions and consider which of the two prototypes your code is calling). Is it definitely a valid type to use in a string concatenation?
@rameshsinghppc once you have made some changes you think will fix the CI please say so in a comment.
@rameshsinghppc once you have made some changes you think will fix the CI please say so in a comment.
@richvdh No sir, It's contributor's responsibility to resolve all the issues. This is my first open source contribution, so I don't know the steps for the open source contribution. I want to do open source contribution and learn through that. One thing also I know that reviewer are very busy they don't have time for bad pull request but I am not getting any other ways through that I can learn open source contribution. If you think my pull request is causing some problems for the maintainers then I am really sorry for that, If you want then I can close this PR.
@richvdh Few days back I made some changes in the code, please re- run the CI.
@richvdh Today I have updated the code with eslint and test cases passed.
Please re-run the CI.
@rameshsinghppc there's no need to merge develop every day.
Thanks for your contribution. I'm going to close this for now as the changes requested haven't been made. If you're interested in continuing to work on this please let us know and we can reopen the pull request. Thanks!