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

Autosuggest data-* attribute-based selectors over className based selectors #2140

Closed
macobo opened this issue Oct 30, 2020 · 8 comments
Closed
Labels
enhancement New feature or request feature/toolbar Feature Tag: Toolbar stale

Comments

@macobo
Copy link
Contributor

macobo commented Oct 30, 2020

Is your feature request related to a problem?

Creating actions requires some technical know-how and is quite involved.

I've now been in two interviews where the automatically suggested selector were not great:

  1. In one, they used inline styling and had dynamically generated class names. They used data-testid attributes internally for testing which could be used.
  2. In another, data-test-foobar was the attribute they were hoping to be used

Describe the solution you'd like

If element has unusual data-attributes, suggest those over class-based ones.

We already seem to have a special case for one data-attribute, data-attr but we should expand it for other data attributes as well. https://github.com/PostHog/posthog/blob/5729fb5/frontend/src/toolbar/utils.ts#L59-L62

Additional context

cc @ukupat and @andrestaht

Thank you for your feature request – we love each and every one!

@macobo macobo added enhancement New feature or request feature/toolbar Feature Tag: Toolbar labels Oct 30, 2020
@tino
Copy link

tino commented Nov 9, 2020

This would be great. We are using Cypress for testing, and thus have a lot of elements annotated with data-cy="...", also because of 1.

These attributes used to show up but don't do anymore?
In browser:
image

In Posthog:
image

@timgl
Copy link
Collaborator

timgl commented Nov 9, 2020

@tino are you self-hosting or on app.posthog.com?

@tino
Copy link

tino commented Nov 9, 2020

@timgl Self-hosting

@timgl
Copy link
Collaborator

timgl commented Nov 10, 2020

@tino The reason for this dat this attribute is on an input element. We try to be really strict around the data we capture from input elements (only name, id and class attributes), as other companies have messed this up in the past. If you put the attribute on the div it will be captured. Sorry about that!

@macobo
Copy link
Contributor Author

macobo commented Nov 12, 2020

@mariusandra had a look at adding this to our fork at https://github.com/mariusandra/simmerjs/tree/local-release.

One thing I had questions on was that we're not following w3c-s specificity ruleset - can you give context on why we dropped that?

@mariusandra
Copy link
Collaborator

@macobo if I remember correctly, it's because a data-attr means something else for us. We consider it similar to a #id. Without forcing it into higher priority, the selector matching logic would find something else and potentially ignore the data-attr.

@posthog-bot
Copy link
Contributor

This issue hasn't seen activity in two years! If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in two weeks.

@posthog-bot
Copy link
Contributor

This issue was closed due to lack of activity. Feel free to reopen if it's still relevant.

@posthog-bot posthog-bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature/toolbar Feature Tag: Toolbar stale
Projects
None yet
Development

No branches or pull requests

5 participants