securingsincity/react-ace

Conditionally setting className removes ace-editor class

Open

#823 geöffnet am 13. Apr. 2020

Auf GitHub ansehen
 (1 Kommentar) (3 Reaktionen) (0 zugewiesene Personen)JavaScript (3.859 Stars) (615 Forks)batch import
bughelp wanted

Beschreibung

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
SchwierigkeitGeschätzte Implementierungsschwierigkeit für neue Contributors, von 1 für sehr kleine Änderungen bis 5 für Expertenarbeit.
2
Geschätzte ZeitEin grober Zeitrahmen, um zu recherchieren, zu implementieren, zu testen und einen Pull Request vorzubereiten.
under 1 hour
AktivitätsstatusWie verfügbar das Issue gerade wirkt: frisch, aktiv, stale, blockiert oder wartet auf Maintainer-Input.
fresh
KlarheitWie klar das Issue die erwartete Änderung, Akzeptanzkriterien und den nächsten Schritt erklärt.
clear
Voraussetzungen
basic React knowledge
EinsteigerfreundlichkeitEin Score von 1 bis 100, der schätzt, wie zugänglich dieses Issue für First-Time Contributors ist.
80
Research-Richtung
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.