Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable editing others comments #163

Merged
merged 2 commits into from
Nov 14, 2020
Merged

Disable editing others comments #163

merged 2 commits into from
Nov 14, 2020

Conversation

ilmartyrk
Copy link
Contributor

Simple fix to #125 with test

@JohnMcLear
Copy link
Member

@rhansen afaik you were involved in the conversation. Are we okay with the UX of this proposed fix?

@ilmartyrk
Copy link
Contributor Author

ilmartyrk commented Nov 13, 2020

@rhansen @JohnMcLear, I went a little further don't merge yet, will also disable deleting and will hide action buttons from UI for others comments, I'll soon push when everything is done and tested

@ilmartyrk
Copy link
Contributor Author

@rhansen @JohnMcLear now disabled deleting too and hide/remove action buttons from other users comments and replies, also added tests

@JohnMcLear
Copy link
Member

Please flag as draft until it's ready for me to test/merge, thanks!

@ilmartyrk
Copy link
Contributor Author

@JohnMcLear it's ready

@JohnMcLear
Copy link
Member

btw if you work on the ether:ep_comments repo the tests will auto-run, you should have the sauce labs user/pass, lemme know if not.

@JohnMcLear
Copy link
Member

I also experience

Gritter Error: You must supply "text" parameter. Object { title: "Error", text: null, sticky: true, class_name: "error" }

@JohnMcLear
Copy link
Member

I experience quite a lot of frontend test failure, it could be something with my environment but I am not sure.. It would be helpful if you can pull your branch over to the ether/ep_comments repo and let sauce labs run the tests. Also if you can look at the empty gritter msg that would be useful.

@ilmartyrk
Copy link
Contributor Author

@JohnMcLear all tests pass on my machine, but I've noticed some race-condition kind of failures sometimes. I've also noticed this gritter error thing. And I have no sauce labs user/pass.

@JohnMcLear
Copy link
Member

JohnMcLear commented Nov 13, 2020

test passes in chrome, fails in FF.

@ilmartyrk did you try run frontend tests in FF or just Chrom(ium/e).

@edsaperia
Copy link

edsaperia commented Nov 14, 2020

Editing someone else's comments is unintuitive, for sure, but I kind of understand why you'd make them that way - to keep the feeling of everything being collaborative and fluid.

As an alternative to fully signed & owned comments as being developed here, we could have "notes"; instead of a comment being a set of linear posts, it could be a little collaborative scratchpad that anyone can edit. Author colours can let it still have a "chat" feel.

@ilmartyrk
Copy link
Contributor Author

@JohnMcLear passing on chrome and FF, I am using Ubuntu 18, Chrome Version 86.0.4240.193 (Official Build) (64-bit) and Firefox 82.0.2 64-bit

@JohnMcLear JohnMcLear merged commit 501e2b2 into ether:master Nov 14, 2020
@JohnMcLear
Copy link
Member

Peep errors in travis logs.

@JohnMcLear
Copy link
Member

@ilmartyrk dude plz fix tests! Tnx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants