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

Show question & reply in the code #109

Open
onemen opened this issue Dec 28, 2018 · 9 comments
Open

Show question & reply in the code #109

onemen opened this issue Dec 28, 2018 · 9 comments

Comments

@onemen
Copy link
Contributor

onemen commented Dec 28, 2018

See screenshot of a patch i'm working on
codeponder

@onemen
Copy link
Contributor Author

onemen commented Dec 28, 2018

form to add comment
codeponder-form

@paean99
Copy link
Contributor

paean99 commented Dec 30, 2018

The PR you did was so nice. it wasn't complete, but i am so happy to see it! Thank you very much (that was very near from what i was envisioning) and great work! :)

@paean99
Copy link
Contributor

paean99 commented Dec 31, 2018

I have been experimenting with the editor and i think it is a good direction to go, but it is not complete and do need some iterations. Here are some of my thoughts.

  • pro: putting the questions inside the code gives them more strength and meaning.
  • con: the more the questions and replies, the less the code. It can very quickly grow to a state where the code is completely unreadable. There is a conflict between the questions and the code for the main seat. It becomes worse since the code page can accommodate multiples questions.
  • solution:
    • The code should have the main spot and only give the secondary seat to the question that the user decides to focus on.
    • The questions can be, by default, toggleable. Possibly use a local toggle state for each of them for a first try. I am not convinced that it deserves a database column for it. As for what shows inside the code on page load, one can show a marked one line with the first words of the question giving a way to interact with the toggle or something similar that is less weighty.
    • And given that there is the possibility of multiples questions for a similar code snippet, one can revisit the figma theme for a way to show and interact with the number of questions for a selection.
    • Also the replies shouldn't be inside the code. Maybe put them on a contextual sidebar (possibly sliding from the right) or another better solution.
    • The mouse selection for the lines numbers is not working well with this UI. It should be revised.
    • I have doubts about this one, but i will put it here nonetheless. Reduce the page to accommodate only one question and its replies. The router would go for something like this: /post/{commitID}/{path} to /post/{questionID}/{commitID}/{path} or possibly to /question/{questionID}/{commitID}/{path}

@onemen
Copy link
Contributor Author

onemen commented Dec 31, 2018

I agree.
my plans are:

  • cleaning the code and make sure the components render currently.
  • automatically add question/reply when the submit button click
  • add toggle button and navigation button to hide single/all questions/replies, view below the code (or side by side)
  • add new page to show only questions/replies, in this page we need to have option to view questions/replies in code.
  • and so on...

once we have the all the component working adding UI will be easier. (I hope)
I'm also planing to replace the simple text-area with component with Markdown supported.

@benawad
Copy link
Owner

benawad commented Dec 31, 2018

Making the questions hidden by default is definitely the way to go.

To simplify things, I like the idea of preventing overlapping questions. If this becomes a thing people really want then we can come back to it.

I kinda like the idea of being able to reply to questions and see the replies without leaving the page and having that as something that toggles underneath the question. But that may only be feasible for short threads. /question/{questionId} makes sense, but how do you see the questionId and commentId working?

@paean99
Copy link
Contributor

paean99 commented Dec 31, 2018

I'm also planing to replace the simple text-area with component with Markdown supported.

+1 for that. I am convinced that the code addition should be done with as simple form as possible.
A text area component with markdown is no doubt the simpler solution. I will nonetheless share this alternative that i found being used very easily by the Prism-renderer creator in some of his examples: https://github.com/satya164/react-simple-code-editor
And here is one of his examples: https://codesandbox.io/s/00o4wx0jqv
Look very simple and simplicity is the goal of the package as they refuse some features to avoid complexity.

but how do you see the questionId and commentId working?

A first draft proposal for some of the possible pages (order not relevant):

    1. /question/{questionId}/{commitID}/{path}. Would be a clone of the actual /post/{commitID}/{path}, but only for a question and its replies. And only for a code file (for the MVP), and no ability to browse the repo (?) Database: 'code_review_question' and 'question_reply'
    1. /repo/explorer/{commitID}/{path}. Similar to actual /post/{commitID}/{path} and with possibly the figma numbers on the right showing how much questions in folders/files/codeLine and link to them. It would also give the possibility to create new questions. 'postID' would be hidden and only 'commitID' would be showed to the user. Database: 'code_review_post' to browse/create repo; 'code_review_question' to create questions.
    1. /repo/questionList/{commitID}. Simple list of questions and their status (closed, open, ...) or category (bug, ...) and linking to page "i." Database: 'code_review_post' to get the postID from the commitID and 'code_review_question' for the list

One detail to consider for the pages: At this time the routing is done with the commitID as a 'namespace', but it is kind of ugly. One should consider using some kind of SEO-friendly URL, like slugs. The commit message and the repo name can be considered for that.

@benawad
Copy link
Owner

benawad commented Dec 31, 2018

i. sounds good, and we can always link back to the repo from that page
ii. how do you see users coming to this page
iii. I'm thinking of having this be on the /post/{postId} page

We don't need the commitId in the url at all, we can just use the postId/questionId. But that's still ugly, so I think creating a slug of some kind is a good idea.

@paean99
Copy link
Contributor

paean99 commented Dec 31, 2018

ii. how do you see users coming to this page

There are different alternatives.

  • Redirect to it on signup to create the first question after the pick repo page.
  • The already existing 'create question' button on the navbar should redirect to it (after pick repo?).
  • possibly a link/button on the i. page in a future version where one can have multiples files for one question.

iii. I'm thinking of having this be on the /post/{postId} page

+1. Just used the /repo/ for ease of explaining the idea.

@onemen
Copy link
Contributor Author

onemen commented Jan 4, 2019

I hope to push an update to pr #115 tomorrow

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

No branches or pull requests

3 participants