Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text Nodes Cannot Be Selected When Siblings Are Element Nodes #11

Open
sshaw opened this issue Feb 20, 2017 · 5 comments
Open

Text Nodes Cannot Be Selected When Siblings Are Element Nodes #11

sshaw opened this issue Feb 20, 2017 · 5 comments

Comments

@sshaw
Copy link

sshaw commented Feb 20, 2017

Given this: screen shot 2017-02-19 at 9 37 15 pm

"866 Results" cannot be selected. If I wrap it in a span, it can be.

.search-options__actions is floated right.

@bensampaio
Copy link
Owner

@sshaw did you wrap it with a span using the browser inspect element or did you do it in your HTML code?

Is it possible to see this page?

@sshaw
Copy link
Author

sshaw commented Feb 22, 2017

Page is not public but this demonstrates the issue:

<!DOCTYPE html>
<html lang='en'>
    <head>
        <title>Foo</title>
    </head>
    <body>
        <h1>Foo</h1>
        <div style="width:300px; margin: auto">
            <div style="float:right">
                <button>Filter</button>
            </div>
            866 results
        </div>
    </body>
</html>

Adding a span to the source fixes it.

I haven't looked at your code but from the markup it looks like it requires an element with a .chrome-extention-wi-element class. Which a text node cannot have.

@bensampaio
Copy link
Owner

@sshaw I understand the problem now and indeed my code will never detect this case. In my opinion, having loose text nodes is never a good practice so I would say that adding a <span> to it is the correct solution. This way this element can easily be styled (if needed) and you won't get white space around the text as shown in the picture you uploaded.

@sshaw
Copy link
Author

sshaw commented Feb 27, 2017

my code will never detect this case...

What's the issue?

In my opinion, having loose text nodes is never a good practice so I would say that adding a to it is the correct solution.

Heh. There's nothing wrong with loose text, it's definitely not a bad practice :neckbeard:.

@bensampaio
Copy link
Owner

What's the issue?

This extension looks for terminal elements (or elements without children) and allows you to select those elements and see their properties. Otherwise you would be able to select a lot of undesirable elements that most people don't even know are there. For your specific case the div that contains "866 results" is not terminal. It contains another div as children so it will never be selectable, but the child div will.

Text nodes cannot have a class so the only possibility here would be to also allow elements that contain text nodes to be selected. However, I think this would be confusing because the parent element size won't necessarily be the size of the clicked text.

Heh. There's nothing wrong with loose text, it's definitely not a bad practice :neckbeard:.

I didn't mean to say there is something wrong with it but from my experience they never make anyones life easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants