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

Parsing Exception: CSS selector query doesn't get attribute value #556

Closed
ngdangtu-vn opened this issue Jan 19, 2024 · 4 comments · Fixed by #558
Closed

Parsing Exception: CSS selector query doesn't get attribute value #556

ngdangtu-vn opened this issue Jan 19, 2024 · 4 comments · Fixed by #558
Labels
bug Something isn't working

Comments

@ngdangtu-vn
Copy link
Contributor

ngdangtu-vn commented Jan 19, 2024

Version

2.0.3

Platform

any

What steps will reproduce the bug?

The issue was submitted in discord: https://discord.com/channels/794537085641818124/1197989006789595156/1197989006789595156

  1. Add _data.yml
metas:
  title: "=title"
  description: "=summary"
  image: "$.feed-image"
  1. Add content that has this HTML element: <img alt="Reading tracker app with Airtable and Deno Fresh, showing the Airtable and Deno logos." class="feed-image" src="/img/posts/reading-tracker-post-3-large.webp" width="1000">

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior?

The reporter expects to have the src set in place of meta og:image.

What do you see instead?

Nothing. the behaviou is correct. But it doesn't make sense for users.

Additional information

The source of problem: https://github.com/lumeland/lume/blob/main/core/utils/data_values.ts#L26

This is an UX bug. This happens due to the convention only return the innerHTML. If it is a void element, nothing will returns. The design needs improve or to be removed as it makes users confused. Need further discussion about this feature.

@ngdangtu-vn ngdangtu-vn added the bug Something isn't working label Jan 19, 2024
@ngdangtu-vn
Copy link
Contributor Author

ngdangtu-vn commented Jan 19, 2024

So far we have 3 cases:

  1. Only need children text node, for instance h1, p
  2. Only need specific attribute, for instance img with [src]
  3. The element provides rich data to use: a with [href] and child node

I also think about the nested element. But I don't think we need to support it. Users should point it directly to the element contains the info.

My solution could break the meaning of selector query. For example we have:

  1. $.feed-image will always get text node
  2. $.feed-image[src] will always get value of src attribute

It is pretty close to selector query syntax but act different as all query return HTMLElement. Another problem is some user may go very creative by adding [src^="https:"]... But looks on bright side, this syntax ensures a stricter query.

@oscarotero
Copy link
Member

I see the problem. The solution of allowing the attribute name is a good idea, but [src] is confusing because it's a valid selector. I'd use a different syntax. An idea: the attr() function . For eaxmple:

$.feed-image attr(src)

Other solution is to make it more automatic:

  • If the selector is to get an URL, get the value from href or src attribute.
  • If not, get the value from the element content.

@ngdangtu-vn
Copy link
Contributor Author

If the selector is to get an URL, get the value from href or src attribute.

I have the 3rd case where we wouldn't know what user may need:

  1. The element provides rich data to use: a with [href] and child node

It could be child text node from the element or it could be href.

I don't mind the attr() css function. Mind if I do this PR?

@ngdangtu-vn ngdangtu-vn changed the title Parsing Exception: Parsing Exception: CSS selector query doesn't get attribute value Jan 20, 2024
@oscarotero
Copy link
Member

Sure! PR is very appreciated 👍

ngdangtu-vn added a commit to ngdangtu-vn/lume that referenced this issue Jan 20, 2024
Tasks:
- improve `getDataValue()` function
- add test cases
- update CHANGELOG
oscarotero added a commit that referenced this issue Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants