-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature/ New forum page: support v2 notes #899
Conversation
Here's what a forum page currently looks like: |
Thanks for sharing it!!! It is a lot of progress!
|
Now a message will be displayed on the new forum page: and the old forum page: Currently only members of the TMLR/Editors_In_Chief group will see the message, but that could be expanded in the future. |
As I mentioned in the daily meeting, this PR is ready for final review and testing. I believe all the outstanding issues have been fixed: https://docs.google.com/document/d/1Z38PWpTCLwYP2JKGcAFAfUyWNgvsP6kHbm_quezIOT4/edit#heading=h.ia2l6konfdfb I'm hoping we can get this merged in the next few days and out to TMLR beta testers soon. |
That's great, I will take a look. It would be nice if you can add a section to the documentation that explains the new features of the forum so we can share it with the TMLR users. |
Yeah, that's a good idea. |
When I click on "View new forum", it stays on the same page, the current forum. I'm impersonating one of the EICs. |
What about showing the message at the top of the page? showing it before the replies implies that only the below section has changed and not the complete page. |
I left more feedback in the document. |
The docs page has been published: https://docs.openreview.net/getting-started/using-the-new-forum-page |
The documentation looks good, I would add more docs about the rest of the page, for example:
They might be too obvious for us but I'm not sure about the users. |
We have the Ok from TMLR to enable this, they are OK enabling for all the users. |
OK, I can add those items to the documentation. Since that is in a separate repo however is there anything preventing this PR from being merged? |
@@ -825,6 +825,7 @@ module.exports = (function() { | |||
editSignatureInputValues = [user.profile.id]; | |||
} | |||
const editToPost = constructEdit({ formData: { editSignatureInputValues,editReaderValues }, noteObj: { ...note, ddate }, invitationObj: invitation }) | |||
console.log(editToPost) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zbialecki remove this console.log
This PR implements initial support for v2 notes and edits in the new forum page. No new features were added to the forum-new, the changes that were made just get the existing feature working with the new data structures. This PR should not affect any of the legacy forum pages or other parts of the site.
Testing
After running the openreview-py journal test, go to the TMLR group and click one of the forum links.
Replace
/forum
in the url bar with/forum-new
to load the new forum page