securingsincity/react-ace

Conditionally setting className removes ace-editor class

Open

#823 opened on Apr 13, 2020

View on GitHub
 (1 comment) (3 reactions) (0 assignees)JavaScript (3,859 stars) (615 forks)batch import
bughelp wanted

Description

Problem

When I am conditionally setting the className prop for an error - the editor position changes and breaks the UI.

For example: I set a className for an error. Once there is no longer an error, I am passing in an empty string. When inspecting the DOM, I noticed that the ace-editor class is getting removed.

After looking at the source code, I found this section which I believe is the culprit. If I am reading it correctly, I believe if there was a previous class name and the new class name is falsy, it would remove one of the libraries set class names (ace-editor).

Sample code to reproduce your issue

Conditional className prop:

className={error ? classes.error : ""}

Steps on how I am able to get this issue:

  • Type in editor (starts with no className)
  • Submit w/ error (updates className to error)
  • Type more in editor (updates className to "")
  • Submit w/ another error (updates className to error)

Contributor guide

Tech stack
javascriptreact
Domain
frontend
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
under 1 hour
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
fresh
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
basic React knowledge
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
80
Research direction
The issue is in the react ace component at line 313 of ace.tsx. Examine the logic where className is set: it likely overwrites the default 'ace editor' class when the new className is falsy. Research how to conditionally merge the className prop with the base class, e.g., using template literals or a classnames utility to avoid removing the required class.