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

Feature/ New forum page: support v2 notes #899

Merged
merged 67 commits into from
Jul 6, 2022
Merged

Conversation

zbialecki
Copy link
Contributor

@zbialecki zbialecki commented Mar 11, 2022

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

@zbialecki
Copy link
Contributor Author

zbialecki commented Mar 11, 2022

Here's what a forum page currently looks like:

@melisabok
Copy link
Member

Thanks for sharing it!!! It is a lot of progress!

  • I would keep a single path /forum with the new implementation.
  • I like how you show the multiple invitations in the replies. We need to figure out something for the forum note.

@zbialecki
Copy link
Contributor Author

zbialecki commented Jun 24, 2022

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.

@zbialecki
Copy link
Contributor Author

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.

@melisabok
Copy link
Member

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.

@zbialecki
Copy link
Contributor Author

Yeah, that's a good idea.

@melisabok
Copy link
Member

melisabok commented Jun 28, 2022

When I click on "View new forum", it stays on the same page, the current forum. I'm impersonating one of the EICs.

@melisabok
Copy link
Member

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.

@melisabok
Copy link
Member

I left more feedback in the document.

@zbialecki
Copy link
Contributor Author

The docs page has been published: https://docs.openreview.net/getting-started/using-the-new-forum-page

@melisabok
Copy link
Member

The documentation looks good, I would add more docs about the rest of the page, for example:

  • Explain the three state buttons next to each reply.
  • Explain the linear view, where the reply has a link to the parent.
  • Explain the new layout of a note, where is the invitation name, signature, real signature, edit date, readers, etc.
  • Explain the edit dropdown to select the edit to add.

They might be too obvious for us but I'm not sure about the users.

@melisabok
Copy link
Member

We have the Ok from TMLR to enable this, they are OK enabling for all the users.

@zbialecki
Copy link
Contributor Author

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?

@melisabok melisabok merged commit 4be2942 into master Jul 6, 2022
@melisabok melisabok deleted the feature/forum-page-v2 branch July 6, 2022 22:19
@@ -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)
Copy link
Member

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

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.

None yet

5 participants