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

10 statements out of videos #777

Draft
wants to merge 2 commits into
base: staging
Choose a base branch
from
Draft

Conversation

vguen
Copy link
Contributor

@vguen vguen commented Mar 16, 2021

@vguen vguen requested a review from Betree March 16, 2021 14:05
@vguen vguen self-assigned this Mar 16, 2021
@vguen vguen marked this pull request as draft March 16, 2021 14:05
@vguen
Copy link
Contributor Author

vguen commented Mar 16, 2021

Early results, still need to fix:

  • some statements are duplicated when displayed
  • cannot use CommentForm for now

@vguen
Copy link
Contributor Author

vguen commented Mar 16, 2021

@Betree to be tested, staging need to have CaptainFact/captain-fact-api#309, is there a way to test both of those branches ?

@Betree
Copy link
Member

Betree commented Mar 17, 2021

@Betree to be tested, staging need to have CaptainFact/captain-fact-api#309, is there a way to test both of those branches ?

No, unfortunately matching branches are not configured for CaptainFact CI (that'd be a great addition). I'll test this one locally!

@Miragide
Copy link
Collaborator

Miragide commented Jun 8, 2021

@vguen @Betree what can we do here?

Remove what's make it fail for now ...

Add filters in statement query

Add default filter for statements query

Add link to go to statement in video debate

Add speaker picture to statement

Add play icon on video link

Hide CommentForm for now ...

Add comments filter on statements

Remove unused file

Remove unused effects

Remove logged

Remove unused functions

Remove unused import

Remove unused imports

Rename constant

Remove comments' effect since they are unused for now...

Remove unused code
@Betree Betree force-pushed the 10_statements_out_of_videos branch from d9e274a to b526f0c Compare June 19, 2021 12:54
@Betree Betree force-pushed the 10_statements_out_of_videos branch from 309e683 to 1ed6de8 Compare June 19, 2021 13:42
Copy link
Member

@Betree Betree left a comment

Choose a reason for hiding this comment

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

I rebased and pushed some fixes and enhancements. The core feature works, it's not far from being ready to merge. We still need to iterate on the design to have something cleaner, but I don't have the bandwidth to do it myself at the moment.

image

@Miragide
Copy link
Collaborator

@vguen souhaites tu finaliser ?

@vguen
Copy link
Contributor Author

vguen commented Sep 30, 2021

@Miragide oui je souhaite reprendre ce développement. 🙂

@Miragide
Copy link
Collaborator

Miragide commented Oct 5, 2021

Merci @vguen
J'avais raté ta réponse, donc j'ai peut-être généré des erreurs dans tes notifications, désolée.

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.

Page(s) to show statements out of videos, statements news feed
3 participants