Skip to content

fix: Unexpected Behavior when Toggling Italics in Text Editor and improve clarity of formatting status#1506

Merged
mattinannt merged 3 commits intoformbricks:mainfrom
moheyt:toggling-italics-in-text-editor
Oct 30, 2023
Merged

fix: Unexpected Behavior when Toggling Italics in Text Editor and improve clarity of formatting status#1506
mattinannt merged 3 commits intoformbricks:mainfrom
moheyt:toggling-italics-in-text-editor

Conversation

@moheyt
Copy link
Copy Markdown
Contributor

@moheyt moheyt commented Oct 29, 2023 •
edited
Loading

…editor

What does this PR do?

This pull request addresses the reported issue with the unexpected behavior when toggling italics in the text editor. Additionally, it includes an enhancement to improve the clarity of formatting status for the users.

Changes Made:

1. Bug Fix: Unexpected Behavior When Toggling Italics:
Modified the code handling the "italic" button's click event to prevent it from unintentionally triggering bold formatting when deactivating italics. The click event now toggles the "italic" formatting without conditional checks for bold.

2. Enhancement: Clarity of Formatting Status:
Introduced a visual indicator for the "bold" formatting. When "bold" formatting is active, the "bold" button's appearance changes to provide a clear visual cue to the user. When "bold" formatting is not active, the button reverts to its default appearance.

Benefits:

  1. Fixes the unexpected behavior when toggling italics, ensuring a smoother user experience.
  2. Enhances user interface clarity by providing a visual indicator for the "bold" formatting, reducing user confusion.

Fixes #1504

Recording.2023-10-30.010955.mp4

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change adds a new database migration
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read How we Code at Formbricks
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran pnpm build
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues
  • First PR at Formbricks? Please sign the CLA! Without it we wont be able to merge it 🙏

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Formbricks Docs if changes were necessary

@vercel
Copy link
Copy Markdown

vercel bot commented Oct 29, 2023

@moheyt is attempting to deploy a commit to the formbricks Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 29, 2023 •
edited
Loading

Thank you for following the naming conventions for pull request titles! 🙏

@github-actions github-actions bot added the bug Something isn't working label Oct 29, 2023
@review-agent-prime
Copy link
Copy Markdown
Contributor

review-agent-prime bot commented Oct 29, 2023

The changes in this PR look good overall. However, I have a few suggestions to improve the code:

  1. In the ToolbarPlugin.tsx file, you have added a condition to add a class to the button based on whether it is active or not. This is a good approach, but it can be made more readable by using a function to generate the class name. Here is a suggestion:
function getButtonClass(isActive: boolean): string {
  return isActive ? "bg-subtle active-button" : "inactive-button";
}

Then, you can use this function in your code like this:

className={getButtonClass(isBold)}
  1. In the stylesEditor.css file, you have added styles for the active-button and inactive-button classes. It's a good practice to add a comment above each class to briefly explain what it does. This will make it easier for other developers to understand your code. Here is a suggestion:
/* Style for active buttons */
.active-button {
  color: #000000;
}

/* Style for inactive buttons */
.inactive-button {
  color: #777; 
}
  1. It's a good practice to always add a newline at the end of your files. This is because some tools, especially those that are used to concatenate files, may not work properly if a file doesn't end with a newline. So, I suggest adding a newline at the end of your stylesEditor.css file.

@Dhruwang Dhruwang self-assigned this Oct 30, 2023
@vercel
Copy link
Copy Markdown

vercel bot commented Oct 30, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
formbricks-cloud ⬜️ Ignored (Inspect) Visit Preview Oct 30, 2023 3:35am
formbricks-com ⬜️ Ignored (Inspect) Visit Preview Oct 30, 2023 3:35am

Dhruwang
Dhruwang approved these changes Oct 30, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@Dhruwang Dhruwang left a comment

Choose a reason for hiding this comment

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

@moheyt Works Well, Thank you for the fix 😊💪. I just added similar fix to Bold button as well !

@moheyt
Copy link
Copy Markdown
Contributor Author

moheyt commented Oct 30, 2023

thanks @Dhruwang can you please add hacktoberfest label into this?

@Dhruwang Dhruwang added hacktoberfest complete these issues to gather points for Hacktoberfest formtribe-2023 labels Oct 30, 2023
@moheyt
Copy link
Copy Markdown
Contributor Author

moheyt commented Oct 30, 2023

@Dhruwang thanks bro means alot.

@moheyt moheyt requested a review from Dhruwang October 30, 2023 05:00
@mattinannt mattinannt added this pull request to the merge queue Oct 30, 2023
Merged via the queue into formbricks:main with commit 8d9b676 Oct 30, 2023
kevinkong91 added a commit to kevinkong91/formbricks that referenced this pull request Oct 30, 2023
@kevinkong91
Merge branch 'main' into dev
3f28fac 
* main: (28 commits)
  chore: Add Table of Contents to README (formbricks#1427)
  fix: account deletion failing issue (formbricks#1509)
  fix: remove welcome card from email preview (formbricks#1495)
  fix(bug): default role implemented (formbricks#1524)
  fix: changing description of Code Action (formbricks#1522)
  refactor: Migrate activity service (formbricks#1471)
  fix: Error in Docs navigation formbricks#1518 (formbricks#1521)
  feat: dynamic title and description (formbricks#1459)
  fix: Spelling Errors (formbricks#1517)
  fix: added scrollbar whenever overflowed in the settings/profile page (formbricks#1498)
  fix: long url not getting reset after closing modal (formbricks#1502)
  fix: Unexpected Behavior when Toggling Italics in Text Editor and improve clarity of formatting status (formbricks#1506)
  fix: zod pin validation failing (formbricks#1507)
  fix: Error message on Login not shown  (formbricks#1508)
  fix: downgrade nextjs to fix error with react email (formbricks#1516)
  chore: downgrade next version in formbricks-com (formbricks#1513)
  feat: picture selection question (formbricks#1388)
  feat: formtribe leaderboard update as per today (formbricks#1505)
  fix: Added if statement for preventing use of reserved word in question ID (formbricks#1435)
  fix: Disabling Welcome Card leads buggy preview (formbricks#1320)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Dhruwang Dhruwang Awaiting requested review from Dhruwang

Assignees

@Dhruwang Dhruwang

Labels

bug Something isn't working hacktoberfest complete these issues to gather points for Hacktoberfest

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG][100 Points] Unexpected Behavior when Toggling Italics in Text Editor

3 participants

@moheyt @Dhruwang @mattinannt