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 (615 forks)batch import
bughelp wanted

Repository metrics

Stars
 (3,859 stars)
PR merge metrics
 (No merged PRs in 30d)

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

Research direction
Inspect the code at src/ace.tsx around line 313 to understand how className is merged. Ensure that the ace editor class is always applied regardless of the className prop.
Tech stack
reactjavascript
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.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
Active
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
Clear
Prerequisites
ReactJavaScriptGit
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
80