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

List of user's changeset comments #4248

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AntonKhorev
Copy link
Collaborator

Add user's changeset comments next to user's diary comments.

image

Pagination is reused along with traces list.
Localization strings are mostly kept in diary comments section for now.

Copy link
Member

@tomhughes tomhughes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should merge note comments under this interface eventually, and make the current user notes view just show notes they opened and not every comment - maybe as part of the planned restructuring of notes to make notes less linked to the initial comment.

config/locales/en.yml Outdated Show resolved Hide resolved
config/routes.rb Outdated Show resolved Hide resolved
@mmd-osm
Copy link
Contributor

mmd-osm commented Sep 14, 2023

We seem to have some existing work on this topic: #1642
Anything which could be reused from there?

@AntonKhorev
Copy link
Collaborator Author

@mmd-osm you have to look why #1642 failed. The reasons in its comments are:

  • "offset/limit based pagination" - I waited for the maintainers to implement whatever they consider to be a correct pagination. Eventually this happened and now I'm copying it.
  • "it's quite large" - I'm splitting this thing in steps and doing one functional step + one refactoring step. This particular pull request at this moment is in we want more refactoring state.

@AntonKhorev AntonKhorev force-pushed the comments branch 2 times, most recently from b85d37b to 5023a7f Compare September 15, 2023 13:44
@AntonKhorev
Copy link
Collaborator Author

I wonder if we should merge note comments under this interface eventually, and make the current user notes view just show notes they opened and not every comment - maybe as part of the planned restructuring of notes to make notes less linked to the initial comment.

Adding note comments here is an obvious extension, however adding note actions is not as obvious. Note comments are currently tied to actions such as closing and reopening. This is something that the planned restructuring completely ignores. If I close a note without writing anything, is it a comment that should be shown here?

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

Successfully merging this pull request may close these issues.

4 participants