tagAppendAttributes()'s .cssSelector doesn't match top-level tag

[cpsievert]

I'd expect this to add the (bar = "BAR") attribute, but it currently doesn't:

tagAppendAttributes(div(class = "foo"), .cssSelector = ".foo", bar = "BAR")
#> <div class="foo"></div>

The reason why this doesn't work is that tagQuery() is only able to $find() children of the input tag

tagQuery(div(class = "foo"))$find(".foo")
#> `$allTags()`:
#> <div class="foo"></div>

#> `$selectedTags()`: (Empty selection)

Maybe it would make sense for tagAppendAttributes() to wrap the input up into a "dummy" tag (cc @schloerke)?

[schloerke]

I'd be ok with having all tag wrapper methods changing the starting location to the top elements instead of the first layer of children.

But I do not want a single tag method behaving differently than the other tag methods.

[schloerke]

It's be a breaking change. But at least it's be a consistent change throughout the methods

[cpsievert]

After chatting with @schloerke, we're thinking it'd be nice to have this functionality more of a 1st class feature of tagQuery(). Here are some of the options:

• Add a new tagQuery() method, called something like $search() or $findSelf()?
• Add a new parameter to $find() called something like includeSelf?
• Add a new parameter to tagQuery() called something like cssSelector?
  • This will be more difficult to pull off cleanly

[schloerke]

After another discussion, tagQuery() will add a method like $matches() similar to a vectorized form of Element.matches(cssSelector).

No alterations will be made to tagAppendAttributes(). Users can use $matches() to help determine their desired behavior in two steps.

[cpsievert]

I thought we'd still make the (technically breaking) change to tagAppendAttributes(), but also add matches() for direct tagQuery() usage?

[gadenbuie]

I think we should revisit the decision not to allow tagAppendAttributes() to use the root container.

A part of the appeal of tagAppendAttributes() and similar functions is that provide an intermediate level of access to the internal structure of Shiny and bslib components. From bslib's perspective, tagAppendAttributes() lets us avoid having to add a lot of function arguments to provide access to our component markup and gives intermediate users consistent tools to add classes, set attributes, etc when our default markup isn't quite the right fit for the user.

In this perspective, it's relatively common to create a component and want to modify some part of the tree nearly in place. It's also reasonably common that you would need to use the parent element as an anchor to get direct children of the parent element. In this case, knowing whether or not the root element matches the selector isn't enough, you'd have to include the root element in the selector.

For example, in the following example there's no way to target the global sidebar container (direct child of .card) without also changing local sidebars within the component.

navs_tab_card(
  sidebar = sidebar("global sidebar"),
  nav("One", layout_sidebar(sidebar("local sidebar"), "Page content"))
) |>
  tagAppendAttributes(
    class = "my-class",
    .cssSelector = ".card > .bslib-sidebar-layout"
  )

The current work-around is to apply tagAppendAttributes() one level higher...

div(
  navs_tab_card(
    sidebar = sidebar("global sidebar"),
    nav("One", layout_sidebar(sidebar("local sidebar"), "Page content"))
  )
) |>
  tagAppendAttributes(
    class = "my-class",
    .cssSelector = ".card > .bslib-sidebar-layout"
  )

...but this could end up moving the tagAppendAttributes() to be quite far from the element it's trying to update.

[gadenbuie]

One more thought: I think that following $(el).find() or el.querySelectorAll() makes a lot of sense, but it's undermined slightly by the fact that we're not operating on the full DOM.

In the JavaScript case, you can always el.parentElement or $(el).parent() and move both up and down the DOM tree.

In htmltools, though, we're operating on a trimmed branch of html-like structure that will end up being part of the DOM, but isn't quite yet. For me, this fact is important and makes starting from the root node more desirable.

[schloerke]

If we adjust tagQuery()'s initial selection to not initialized, then we can have the first searching performed including the root element. This would allow for the first $find() call to find top level tags.

This would be a breaking change.

If $selectedTags() is called without a $find() like call, then an error should be thrown.

Suggested behavior:

• tagQuery(div(class="foo", "content")) should have selected tags not initialized
• tagQuery(div(class="foo", "content"))$selectedTags() should throw an error suggesting a call to $allTags()
• tagQuery(div(class="foo", "content"))$find(".foo") should have selected tags equal list(div(class="foo", "content"))
• tagQuery(div(class="foo", "content"))$find(".foo")$find(".foo") should have an empty set of selected tags
• tagQuery(list(div(class="foo", "content")))$find(".foo") should return selected tags equal list(div(class="foo", "content"))
• tagQuery(list(div(class="foo", "content")))$find(".foo")$find(".foo") should have an empty set of selected tags

---

If this was implemented, all of the tagAppend-like methods would automatically adopt this behavior of being able to execute on top level elements.

[gadenbuie]

For the second item, could $selectedTags() return NULL:

• tagQuery(div(class="foo", "content"))$selectedTags() returns NULL because there isn't a $find() to find anything.

[wch]

I don't currently have an opinion about tagAppendAttributes, but for $find() I think it could make sense to have a parameter like self=TRUE.

[schloerke]

Talking with @jcheng5 , he might be up for adopting the proposed behavior within the tag methods (where the .cssSelector can find top level tags) but leaving tagQuery()'s implementation as is.

Will come back to this Issue at a later date