Skip to content

use: document requests and comments#904

Merged
fenekku merged 2 commits intoinveniosoftware:masterfrom
carlinmack:use-requests
Feb 4, 2026
Merged

use: document requests and comments#904
fenekku merged 2 commits intoinveniosoftware:masterfrom
carlinmack:use-requests

Conversation

@carlinmack
Copy link
Copy Markdown
Contributor

❤️ Thank you for your contribution!

Description

Please describe briefly your pull request.

Add documentation which is mostly imported, with permission, from https://repository.cern/docs/review/comments/#locking-the-conversation

image

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

  • I'm aware of the code of conduct.
  • I've created logical separate commits and followed the commit message format.
  • I've targeted the master branch.
  • If this documentation change impacts the current release of InvenioRDM, I will backport it to the production branch following approval or indicate to a maintainer that it should be backported.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.

@carlinmack carlinmack changed the title docs: add use/requests.md use: document requests and comments Feb 3, 2026
@carlinmack carlinmack mentioned this pull request Feb 3, 2026
4 tasks
Copy link
Copy Markdown
Collaborator

@fenekku fenekku left a comment

Choose a reason for hiding this comment

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

👍 (most comments are small) feel free to merge when you are happy with your fixes.


![The popover with the "quote reply" button above a highlighted piece of text](imgs/requests/quote_reply_popover.png)

#### Auto-saved drafts
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sidenote: this is kind of wild I daresay. (sounds like backend work wanted to be skipped haha)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

you mean that it is not a guarantee that it will be autosaved? I think the idea is that if we over promise on this then the user could start using it like google docs or something and we cannot promise that level of reliability. Perhaps we can change the wording here to be less aggressive

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I reduced the number of disclaimers and added a Limitations heading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I was thinking more about the feature itself: we are saving in localstorage (must be) an unfinished comment not associated with user (but presumably with the request and reply?). It seems very unusual to me and brittle (i.e., lots of caveat). But if it exists might as well document it 😄

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@fenekku it's a good point, indeed it's quite brittle, but it's not designed to be a full draft management system. It's more of a last-resort backup that makes users happy when they see that they didn't lose their work. As such, we decided not to make it visible at all in the UI so as to avoid a false sense of security, and to avoid users building a reliance on it. The only way to find out about it is to accidentally get rescued by it or to read about it in the docs.

Adding support for API sync would have introduced a lot of complexity and additional effort (e.g. creating DB models, migrations, service, route handlers, security, etc). We thought most users would lose drafts by accidentally refreshing the page, rather than consciously expecting them to be synchronised to their account, so making these efforts wasn't worth it for now. I hope that's okay!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Got it and I think Carlin's lowered emphasis is the right move here to correlate with the subtlety of it. It will be interesting to see how users perceive it for sure. It's all good.

@carlinmack
Copy link
Copy Markdown
Contributor Author

@fenekku good to merge :) I don't have permissions

@fenekku fenekku merged commit c06cebc into inveniosoftware:master Feb 4, 2026
1 check passed
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