Fix description of several Range methods#38518
Conversation
Josh-Cena
commented
Mar 7, 2025
- Fix Range: compareBoundaryPoints() method examples and explanation unclear #35829
- Fix Range: comparePoint() method return not actually explained #35830
- Fix Range: isPointInRange() method, offset not explained #35840
- Fix Range.extractContents() and Range.surroundContents() describe behaviour incorrectly #35854
| _sourceRange_ to the start boundary-point of `Range`. | ||
|
|
||
| - `sourceRange` | ||
| - : A constant describing the comparison method. Note that the constant names are backwards relative to what actually get compared: the `compareBoundaryPoints()` method compares `this` to `otherRange` (where `-1` means before and `1` means after), but the constant names are written as "`otherRange` to `this`". |
There was a problem hiding this comment.
I find this a very confusing sentence!
One problem is that "comparing A to B" and "comparing B to A" are the same (to me) unless you insist that -1 must mean "before" and 1 must mean "after". So it hurts my brain to read "it compares A to B, not B to A".
I honestly think it would be better just to omit the sentence.
I like the use of this and otherRange in the bullets, I think that helps a lot.
There was a problem hiding this comment.
"-1 means before, 1 means after" is an established convention. You see that in String.localeCompare and the comparator function of sort. Also in general "compare A to B" means "return A - B` for some abstract notion of subtraction.
There was a problem hiding this comment.
I'm just telling you how this sentence reads to me, as a non-expert but not completely ignorant reader. Telling me it should make sense to me doesn't affect that.
There was a problem hiding this comment.
I think if you want to say something about this, it would be better to put it alongside the return value (so it doesn't anticipate), and put it in a note, and say something like:
Note that it is a common convention for APIs that compare A to B to return -1 if A is before B, and 1 if A is after B, but this API reverses that convention.
For example,
END_TO_STARTcompares the end ofotherRangeto the start of this range, but it returns -1 if the start of this range is before the end ofotherRange.
But I think it's as good or better just to be as clear as possible about what this specific API does, and not waste cognitive effort on what it doesn't do.
| deleteContents() returns: | ||
|
|
||
| <p>para</p><p>graph 3</p> | ||
| ``` | ||
|
|
There was a problem hiding this comment.
this is a nice example (and maybe worth keeping anyway) but it would be even better to update the current terrible example (and even better if it were a live sample).
Co-authored-by: wbamberg <will@bootbonnet.ca>
| }); | ||
| ``` | ||
|
|
||
| {{EmbedLiveSample("using_deletecontents", "", "150")}} |
There was a problem hiding this comment.
I think this needs a "Reset" button (https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Page_structures/Live_samples#displaying_a_reset_button) because it's a destructive change. And it would be nice if it logged innerHTML (https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Page_structures/Live_samples#displaying_a_single_entry_log) instead of making me use the devtools.
I actually see some behaviour I don't understand here. If I select some of p1, all of p2, and some of p3:
...then delete:
...then I see what I expect in the devtools:
If I then select everything that's left:
...and delete, then it's all gone:
...but the devtools still shows the old DOM:
devtools bug?
There was a problem hiding this comment.
Update, yes, probably Firefox devtools bug. This doesn't happen in Chrome.
There was a problem hiding this comment.
@wbamberg I originally had the capability of printing innerHTML, but (a) I'd rather encourage people to inspect DOM and keep the live sample seem more plausible (it currently resembles a real text editor) (b) the more markup there is, the more stuff there is to delete, and the UX may become more confusing (for example, if you print the inner HTML of the whole document, then the next time it's going to print itself; if you wrap these three elements inside a container, then it's possible to delete that container via the button, and then you may run into an error).
There was a problem hiding this comment.
yes, it's a fair point, it also occurred to me that you would want to wrap the paragraphs in a container and then the user might delete it.
|
For some reason, the reset button doesn't work—the writing guide one doesn't work either. I don't want to implement a more complicated reset though, like actually re-inserting the DOM. |
The writing guide one works for me in Firefox and Chrome (Chrome is very slow of course but that just seems to be the normal UX these days). |
Though I suppose if it takes several seconds to work it's not really usable anyway. Ugh. In that case tbh you might was well make it a static sample. It's pretty frustrating deleting bits of the content and then having to reload the page if you want to try again. |
|
I don't mind it being laggy on Chrome. I tried and it works well on both Firefox and Safari anyway. I can just actually refresh the whole page if I want to reset. |
wbamberg
left a comment
There was a problem hiding this comment.
👍 thanks for your patience!
* upstream/main: (172 commits) chore: improve code style guide (mdn#38715) fix: typo on `Error.isError()` page (mdn#38754) plural consistency (mdn#38747) fix: auto-cleanup by bot (mdn#38695) Synchronize with BCD v5.7.4 (mdn#38709) Add docs for JS self-profiling API (mdn#37796) Better SameSite docs (mdn#38710) Added missing explanation for Array Literals (mdn#38745) Add a page on CSRF (mdn#38151) Fix description of several Range methods (mdn#38518) Remove extraneous span (mdn#38696) Add a definition for media containers, improve how the media files are defined and Remove wrong information (mdn#38721) Move visited selector guide to CSS selectors module (mdn#38642) Make JSON learning article more technically precise (mdn#38644) Make translate3d() interactive example code valid (mdn#38647) Clarity on Safari support for custom elements (mdn#38727) feat(css): Link to learning doc about text direction (mdn#38719) Fix typo (mdn#38739) move guide to module: inline formatting context (mdn#38637) Fix CSS pseudo-class lists (mdn#38576) ...
* Fix description of several Range methods * Apply suggestions from code review Co-authored-by: wbamberg <will@bootbonnet.ca> * Update index.md * Better example * Update files/en-us/web/api/range/compareboundarypoints/index.md * Add a reset button --------- Co-authored-by: wbamberg <will@bootbonnet.ca>