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

Fix(Client): Clarify Progress Reset Modal #54078

Closed
wants to merge 7 commits into from

Conversation

RavidYael
Copy link

@RavidYael RavidYael commented Mar 13, 2024

Checklist:

Overview

This pull request closes Issue #53959 through the implementation of a new reset modal as discussed, including a mechanism for backward compatibility for languages yet to receive updates for the new modal translations.

Changes

  • Translations: New key-value pairs have been added to translations.json for enhanced clarity in the modal's messaging.
  • Component Architecture: NewResetModal and OldResetModal components have been created, segregating the new modal's functionality from the existing one.
  • Rendering Logic: A ResetModalRenderer component is implemented to selectively render modals based on translation availability.

Backward Compatibility

  • Translation Existence Check: ResetModalRenderer utilizes i18n keys to verify the presence of new modal translations.
  • Fallback to Legacy Modal: In the absence of new translations, the old modal is rendered, preserving the user experience.
  • Progressive Translation Updates: This dual-modal system enables incremental translation updates without disrupting user functionality.

Updated Reset Modal Interface

image

  • Here's how the new reset modal looks in the local development environment, reflecting the latest UI updates as discussed in the issue.

@RavidYael RavidYael requested a review from a team as a code owner March 13, 2024 17:32
@github-actions github-actions bot added platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. scope: i18n language translation/internationalization. Often combined with language type label labels Mar 13, 2024
@RavidYael RavidYael changed the title Fix: Clarify Progress Reset Modal Fix(Client): Clarify Progress Reset Modal Mar 13, 2024
@Sembauke Sembauke mentioned this pull request Mar 18, 2024
18 tasks
@huyenltnguyen huyenltnguyen self-assigned this Mar 18, 2024
Copy link
Member

@huyenltnguyen huyenltnguyen left a comment

Choose a reason for hiding this comment

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

@RavidYael Thank you for the PR.

There are a couple of things we need to address:

  • The changes should be applied directly to the existing modal, meaning there shouldn't be a new one and an old one
  • Modal should be from @freecodecamp/ui, rather than @freecodecamp/react-bootstrap
  • Custom styles need to be minimal (we do want to keep the UI consistent across pages). And if there is a need for custom styles, the CSS code has to be in a CSS file rather than being applied inline

You're welcome to apply the changes, but note that we are in the process of migrating the Modal component (#52759), and we'll need to block this PR until the migration is complete.

@huyenltnguyen huyenltnguyen added status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP labels Mar 21, 2024
@huyenltnguyen huyenltnguyen removed the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Apr 4, 2024
@camperbot
Copy link
Contributor

Thank you for opening this pull request.

This is a standard message notifying you that we've reviewed your pull request and have decided not to merge it. We would welcome future pull requests from you.

Thank you and happy coding.

@camperbot camperbot closed this Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. scope: i18n language translation/internationalization. Often combined with language type label status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify Progress Reset Modal
3 participants