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.

Sign up for GitHub

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

Jump to bottom

Fix #20923: Display Character Limits in Chapter Editor #20999

Merged
merged 14 commits into from
Sep 28, 2024
Merged

Fix #20923: Display Character Limits in Chapter Editor #20999

merged 14 commits into from
Sep 28, 2024

Conversation

TARishabh
Copy link
Collaborator

@TARishabh TARishabh commented Sep 25, 2024 •
edited
Loading

Overview

  1. This PR fixes or fixes part of #[20923].
  2. This PR adds a character limit indicator in the chapter editor for both the chapter title and chapter description fields. This improvement enhances user experience by informing users of the character limits as they type, preventing potential submission errors related to exceeding the allowed character count.
  3. The original bug occurred because the chapter editor did not specify the character limits for the title and description fields, leaving users unaware of the constraints.

Essential Checklist

Please follow the instructions for making a code change.

  • I have linked the issue that this PR fixes in the "Development" section of the sidebar.
  • I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
  • I have written tests for my code.
  • The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
  • I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).

Proof that changes are correct

chapter-character.mp4

PR Pointers

@TARishabh TARishabh requested a review from a team as a code owner September 25, 2024 17:41
@TARishabh TARishabh requested a review from Nik-09 September 25, 2024 17:41
@oppiabot OppiaBot
Copy link

oppiabot bot commented Sep 25, 2024

Assigning @Nik-09 for the first pass review of this PR. Thanks!

@seanlip seanlip mentioned this pull request Sep 26, 2024
Closed
5 tasks
@TARishabh
Copy link
Collaborator Author

Hello @Nik-09, could you please provide insights into the potential causes for the failure of the End-to-End, Lighthouse CI performance, and Acceptance tests / check_e2e_lighthouse_performance_acceptance_workflow_status?

@Nik-09
Copy link
Member

Nik-09 commented Sep 27, 2024

Hello @Nik-09, could you please provide insights into the potential causes for the failure of the End-to-End, Lighthouse CI performance, and Acceptance tests / check_e2e_lighthouse_performance_acceptance_workflow_status?

Hi @TARishabh I looked into the errors and most of it seems like a flake apart from prettier (try to fix using npx prettier . --write). I have re-runned the checks. Also adding devworkflow team leads for visibility. @Vir-8 @Akhilesh-max cc/ @seanlip

Thanks

Nik-09
Nik-09 approved these changes Sep 27, 2024
View reviewed changes
Copy link
Member

@Nik-09 Nik-09 left a comment

Choose a reason for hiding this comment

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

LGTM

@Nik-09 Nik-09 assigned TARishabh and unassigned Nik-09 Sep 27, 2024
@oppiabot oppiabot bot added the PR: LGTM label Sep 27, 2024
@oppiabot OppiaBot
Copy link

oppiabot bot commented Sep 27, 2024

Hi @TARishabh, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

@TARishabh
Copy link
Collaborator Author

@Nik-09 Fixed the Prettier Issue. Thanks !

@HardikGoyal2003 HardikGoyal2003 added this pull request to the merge queue Sep 28, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 28, 2024
@TARishabh
Copy link
Collaborator Author

Hello @HardikGoyal2003 , the PR was removed from the merge queue due to a status check. Could you please advise if there is something I might have missed in my PR? I would appreciate your guidance in determining whether this issue is on my end.

@HardikGoyal2003
Copy link
Member

HardikGoyal2003 commented Sep 28, 2024 •
edited
Loading

Hey @TARishabh This is due to the flakes occurring in the merge, you don't need to worry about these!!

@TARishabh
Copy link
Collaborator Author

Hey @TARishabh This is due to the flakes occurring in the merge, you don't need to worry about these!!

Thanks for the clarification, @HardikGoyal2003! Glad to know it's just the flakes causing the issue.

@seanlip seanlip merged commit 8fdb252 into oppia:develop Sep 28, 2024
67 checks passed
@github-actions GitHub Actions
Copy link

@TARishabh Congratulations on your second merged PR to Oppia! 🎉 🥳
Since you've merged two PRs, you might be eligible to join the Oppia dev team as a collaborator. If you'd like to do that, please fill in this form. Thanks!

Developer onboarding lead : @HardikGoyal2003

@seanlip seanlip mentioned this pull request Oct 3, 2024
Closed
@TARishabh TARishabh deleted the fix-chapter-editor-character-limits branch October 26, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nik-09 Nik-09 Nik-09 approved these changes

Assignees

@TARishabh TARishabh

Labels
PR: LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@TARishabh @Nik-09 @HardikGoyal2003 @JayVivarekar @seanlip @RishabhSharma0007