Fix missing style transfer from existing textarea to hwt containers #19

Dalimil · 2017-09-24T16:35:28Z

This pull request fixes a couple of CSS bugs in the original plugin.

To exactly match our text overlay we must ensure that the two text containers have the same styling. For textareas that already have certain line-height and font CSS properties set, we must ensure that these are copied to our new containers, so that both text overlay perfectly.

This pull request also fixes previously transparent background - for dark (or any non-white) websites, this would modify the appearance of textareas and make them hard to read.

lonekorean · 2017-09-25T20:30:26Z

Thanks for this, I like the idea. Some concerns though. It doesn't play well with existing CSS. For example, on the demo page:

I'd like the solution to be backwards compatible. I don't know if that's feasible to do automatically, or if we put your code behind an option that people enable? Not sure.

Also not sure about which styles are copied from the textarea element and which are not. Just as an example, letter-spacing can cause a text styling mismatch, so we'd probably want to copy it. But there are so many other styles to consider, too. I don't know where the sanity line is.

I'll keep thinking on it, would be sweet to make this work.

lonekorean · 2017-09-25T20:30:54Z

jquery.highlight-within-textarea.css

@@ -29,10 +29,8 @@
 .hwt-input {
 	display: block !important;
 	position: relative !important;
-	margin: 0;
-	padding: 0;
+	margin: 0 !important;


Why was margin left behind?

If the original textarea had any margin set, it would be applied inside the .hwt-container, instead we want to add that amount of margin to the actual .hwt-container (so that it feels like the original textarea element) and remove any margin from the textarea inside

lonekorean · 2017-09-25T20:31:41Z

jquery.highlight-within-textarea.js

+		},
+
+		getContainerDivCssFix: function(currentTextarea) {
+			const textareaStyle = window.getComputedStyle(currentTextarea);


Is there a performance hit with getComputedStyle? Worth caching?

lonekorean · 2017-09-25T20:31:57Z

jquery.highlight-within-textarea.js

@@ -339,7 +371,7 @@
 			this.$backdrop.remove();
 			this.$el
 				.unwrap()
-				.removeClass(ID + '-text ' + ID + '-input')
+				.removeClass(ID + '-content ' + ID + '-input')


Good catch!

Dalimil · 2017-09-25T22:35:59Z

Couple of changes here. I believe users should not try to style .hwt-content class - this is our internal class. Let them style their actual textarea or any classes that they apply to their textarea - we'll simply copy the computed styles from them (font, line-height, etc.).

Once you remove the custom .hwt-content styles from the demo page (style textarea directly instead - simply change the CSS rule selector to update), it should work as expected with the new version.

lonekorean · 2017-09-26T20:18:57Z

I see your reasoning about it being an internal class, but this would still be a breaking change for current implementations. I would only consider breaking backwards compatibility for a new major version release, but I don't think it's ready for that. Still unsure how to determine all the properties that get copied and all the properties that don't.

Dalimil · 2017-09-26T21:05:29Z

Hmmm, I suppose I also have a slightly different use case for the plugin. Currently, the styling of .hwt-content works fine if the highlight function of the plugin is applied right after the page loads. One should then call $('.string-example').highlightWithinTextarea({ highlight: '' }) to remove the highlight, because calling $('.string-example').highlightWithinTextarea('destroy') (with the class/id bug fixed) would completely change the styling. Demo page:

My use case is: I have a couple of textareas (potentially with various CSS styles) on a page and need to start highlighting their text at some point (call the highlight function). What my pull-request is trying to achieve is keeping the visual state the same before (without .hwt-* classes) and after the function is called (with .hwt-* classes).

lonekorean · 2017-09-29T17:23:57Z

Ah, I see. That makes sense.

…crollbar

GrayedFox · 2019-09-05T00:25:39Z

Hi @Dalimil - sorry for bringing up such an old thread - but were you at all successful in applying the highlight without changing the original textfields? I need to do exactly that for this plug-in I'm developing, and I think your approach of styling the textarea based on existing attributes is the way to go.

What did you mean however by "Once you remove the custom .hwt-content styles from the demo page (style textarea directly instead - simply change the CSS rule selector to update), it should work as expected with the new version."?

I ask because after implementing your changes I am seeing the same bug (highlight is no longer matched to text -- the textfield now retains it's original style/position/etc however highlighting is off).

Thanks for your time!

Dalimil · 2019-09-10T22:14:17Z

@GrayedFox I believe I got it to work for my purposes, but it's been such a long time ago I honestly don't remember the code details... : https://github.com/Dalimil/highlight-within-textarea

GrayedFox · 2019-09-11T12:36:29Z

@Dalimil hey thanks for the reply, I ended up getting it working too, it was a matter of getting the padding and border values right (or more specifically, inheriting those from the textarea). Cheers :)

…olve miss-aligned highlightning lonekorean/highlight-within-textarea#19 lonekorean/highlight-within-textarea#21

Dalimil added 2 commits September 24, 2017 16:38

Fix textarea class mismatch

f63ac46

Fix missing style transfer from existing textarea to hwt containers

6d99a03

Dalimil mentioned this pull request Sep 24, 2017

highligting appearing above textarea in bootstrap #16

Open

lonekorean reviewed Sep 25, 2017

View reviewed changes

Dalimil added 2 commits September 25, 2017 22:51

Fix hwt-content CSS styling

31d28c0

Fix cross-browser CSS2Properties object

6ed68cb

Dalimil added 6 commits September 29, 2017 19:38

Fix resizable textareas with initial percentage-based width/height CSS

1be2226

Fix CSS for predefined div styling

6bbe1aa

Fix CSS for variable width and height in resizable textareas

2ce2fee

Fix CSS for variable width and height in resizable textareas with a s…

713a608

…crollbar

Fix margin styling override in Firefox

1beb47e

Fix font-weight transfer

676e299

qrnik mentioned this pull request Nov 17, 2017

Miss-aligned but only sometimes #21

Open

nook24 added a commit to it-novum/openITCOCKPIT that referenced this pull request Sep 30, 2020

Update highlight-within-textarea lib and move it into git repo to res…

2991dbb

…olve miss-aligned highlightning lonekorean/highlight-within-textarea#19 lonekorean/highlight-within-textarea#21

nook24 added a commit to it-novum/openITCOCKPIT that referenced this pull request Sep 30, 2020

Update highlight-within-textarea lib and move it into git repo to res…

767fc49

…olve miss-aligned highlightning lonekorean/highlight-within-textarea#19 lonekorean/highlight-within-textarea#21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix missing style transfer from existing textarea to hwt containers #19

Fix missing style transfer from existing textarea to hwt containers #19

Dalimil commented Sep 24, 2017

lonekorean commented Sep 25, 2017

lonekorean Sep 25, 2017

Dalimil Sep 25, 2017

lonekorean Sep 25, 2017

Dalimil Sep 25, 2017

lonekorean Sep 25, 2017

Dalimil commented Sep 25, 2017 •

edited

Loading

lonekorean commented Sep 26, 2017

Dalimil commented Sep 26, 2017

lonekorean commented Sep 29, 2017

GrayedFox commented Sep 5, 2019 •

edited

Loading

Dalimil commented Sep 10, 2019

GrayedFox commented Sep 11, 2019

Fix missing style transfer from existing textarea to hwt containers #19

Are you sure you want to change the base?

Fix missing style transfer from existing textarea to hwt containers #19

Conversation

Dalimil commented Sep 24, 2017

lonekorean commented Sep 25, 2017

lonekorean Sep 25, 2017

Choose a reason for hiding this comment

Dalimil Sep 25, 2017

Choose a reason for hiding this comment

lonekorean Sep 25, 2017

Choose a reason for hiding this comment

Dalimil Sep 25, 2017

Choose a reason for hiding this comment

lonekorean Sep 25, 2017

Choose a reason for hiding this comment

Dalimil commented Sep 25, 2017 • edited Loading

lonekorean commented Sep 26, 2017

Dalimil commented Sep 26, 2017

lonekorean commented Sep 29, 2017

GrayedFox commented Sep 5, 2019 • edited Loading

Dalimil commented Sep 10, 2019

GrayedFox commented Sep 11, 2019

Dalimil commented Sep 25, 2017 •

edited

Loading

GrayedFox commented Sep 5, 2019 •

edited

Loading