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

[front] feat: display entity contexts on comparison page #1858

Merged
merged 6 commits into from
Dec 21, 2023

Conversation

GresilleSiffle
Copy link
Collaborator

@GresilleSiffle GresilleSiffle commented Dec 4, 2023

related issues #1852


Description

This PR displays the entity contexts in the comparison interface.

The boxes can be collapsed, and this state is saved per entity in the browser local storage.

On small screens and mobile, the two boxes stack on top of each other.

to-do

  • do not display two context box when both selectors display the same entity
  • make the content box collapsible
  • save the collapsed state of contexts in the local storage
  • when two context are displayed, make it clear that context A is related to video A, and context B to video B
  • reload the context text when the UI language changes

Preview

The title of the entity is also displayed before the context to connect more explicitly the entity with its context.

capture

The following captures have been made before the introduction of the entity title.

one entity with context two entities with context two folded contexts
capture1 capture2 capture3
small screens
capture4

Checklist

  • I added the related issue(s) id in the related issues section (if any)
    • if not, delete the related issues section
  • I described my changes and my decisions in the PR description
  • I read the development guidelines of the CONTRIBUTING.md
  • The tests pass and have been updated if relevant
  • The code quality check pass

@GresilleSiffle GresilleSiffle force-pushed the front-add_context_in_comparison branch from 4f7203a to 6769ff9 Compare December 4, 2023 14:45
@GresilleSiffle GresilleSiffle changed the base branch from main to front-add_linktofaq_in_context December 4, 2023 14:46
@GresilleSiffle GresilleSiffle self-assigned this Dec 5, 2023
@GresilleSiffle GresilleSiffle added the Frontend Front-end code of Tournesol label Dec 5, 2023
@GresilleSiffle GresilleSiffle marked this pull request as ready for review December 5, 2023 10:52
@GresilleSiffle GresilleSiffle marked this pull request as draft December 5, 2023 10:56
@GresilleSiffle GresilleSiffle marked this pull request as ready for review December 6, 2023 09:29
@GresilleSiffle GresilleSiffle force-pushed the front-add_context_in_comparison branch from 8e4ebda to df5bf4d Compare December 13, 2023 09:46
Base automatically changed from front-add_linktofaq_in_context to main December 13, 2023 09:47
@GresilleSiffle GresilleSiffle force-pushed the front-add_context_in_comparison branch from df5bf4d to f45c2d9 Compare December 13, 2023 09:52
Copy link
Member

@amatissart amatissart left a comment

Choose a reason for hiding this comment

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

LGTM with minor comments

* corresponding localized entity contexts.
*/
useEffect(() => {
if (initializing.current || isLoading) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this check on initliazing.current is useful here 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I checked and it's not useful, the isLoading check seems to be enough.

frontend/src/features/entity_context/EntityContextBox.tsx Outdated Show resolved Hide resolved
@GresilleSiffle GresilleSiffle merged commit b389f1c into main Dec 21, 2023
6 checks passed
@GresilleSiffle GresilleSiffle deleted the front-add_context_in_comparison branch December 21, 2023 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend Front-end code of Tournesol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants