Portal API that allows bypassing the automatic section scoping in a controlled way

[khamusa]

This is a first attempt at solving the issue with verifying content on toast messages or interacting with floating date pickers that are placed in the root of the body element.

Check the changes to the README.md file for the proposed API.

[khamusa]

When implementing some IAM tests with tabasco, I stumbled upon the need to interact with a floating datepicker element that is outside the form container. We already have use cases for interacting with our toast messages, and I'd rather not use the same portal for every possible component. Instead, we want our assertions to be specific, so we don't risk clicking on a toast message that happens to be on the page at the same time as a datepicker.

So having a single portal configuration makes little sense. I'm willing to extend the API with named portals instead:

Tabasco.configure do |config|
  config.portal(:toast_message) { find("[data-portal-container]") }
  config.portal(:date_picker) { find("[data-datepicker-thingy]") }
end

Then:

class MyForm < Tabasco::Section
  portal :date_picker                           # matches the datepicker portal by name
  portal :alert, type: :toast_message # matches the toast_message portal, but exposes it under a different name
end

Which could then be used as any other section/page objects:

my_form.date_picker.has_content!("loremipsum")
my_form.alert.has_content!("loremipsum dolor sit")

Thoughts?

[Oyster-Moura]

@khamusa

my_form.date_picker.has_content!("loremipsum")
my_form.alert.has_content!("loremipsum dolor sit")

This looks good but what happens if there's multiple matches for each type of portal? Because right now we would get an exception from the find.

Should we allow users to select them?

Tabasco.configure do |config|
  config.portal(:toast_messages) { all("[data-portal-container]") }
end

class MyForm < Tabasco::Section
  portal :alert, type: :toast_messages, select: -> { |all|  all.first } # , select: -> { |all|  all.find("#idk") }
end

Or when setting up the portal we just need to be careful only select one?

[silvabox]

@khamusa love the idea 🎉.

@Oyster-Moura the idea of selecting a portal seems overly complicated to me. I think a portal should only ever resolve to a single element

@khamusa I find the approach to aliasing slightly less intuitive as it introduces the concept of a type. I'm always wary of using 'type' where an alternative might be found, because type is so often used in other contexts and can create ambiguity. If aliasing is going to be a thing (and if you have it once, it's almost inevitable!) then we should perhaps consider a convention that's going to be part of the Tabasco DSL. How about?:

class MyForm < Tabasco::Section
  portal :date_picker                           # matches the datepicker portal by name
  portal :alert, as: :toast_message    # matches the toast_message portal, but exposes it under a different name
end

[silvabox]

I would recommend all errors extend a TabascoError base class that wraps StandardError. That way implementers can write catchall handlers; i.e.:

rescue TabascoError => e
  # etc.

[khamusa]

Yeah, this was me quickly prototyping something 😓 . We even already have that base class defined as Tabasco::Error.

It's a good reminder, though, I'd likely forget it 😆

[khamusa]

@Oyster-Moura for sections, we force users to specify a test_id, and force the usage of find behind the scenes, which will raise an error if multiple occurrences are found. I think this is a desired quality everywhere.

Or when setting up the portal we just need to be careful only select one?

This is precisely what tabasco aims to avoid! If we need to be careful to not mess something up, the framework is failing us! ;)

You raised a relevant point, but rather than enlarging the framework to account for more cases, I'd rather make it more restrictive. The root cause of the issue is the fact a config proc allows the user to locate the portal using arbitrary capybara finders. The fact all can find multiple occurrences is problematic, but, even worse, it does not wait for elements to appear on the page. So this decision we're placing at the core of the framework opens room for flakiness - going in the opposite direction of what we want (a framework that makes you safer without you even realizing it).

Limiting to only test_id for portals does not seem a good choice, though. Imagine our users will employ different UI libraries (with floating elements that often require portal logic), which may or may not be customizable for them to add a test_id in the precise DOM element they need. Datepicker components are often out-of-the-box components that don't offer a lot of customization options.

So I think we should drop the proc approach so we can enforce the use of find, but at the same time give users more find-compatible options to locate their portals:

# no extra arguments, defaults to find("[data-testid='toast_messages']) - consistent with the rest of the framework
config.portal(:toast_messages)

# can be overridden
config.portal(:toast_messages, test_id: :lorem_ipsum)

# other find-compatible finder approaches:
config.portal(:toast_messages, css: "div.portal.nice")
config.portal(:toast_messages, xpath: "...")

I'd go ahead and implement this new approach using test_ids only, knowing the API can be extended later with other finder methods.

WTDYT?

[khamusa]

@silvabox / @Oyster-Moura I've added a second commit with what I believe would be the minimal implementation for this new API.

It currently allows you to define multiple portals and specify a different test_id. It does not allow finding containers using any other strategy, and aliases are not supported either at this stage. Although both features would be simple to add, I'd rather expand the Tabasco interfaces conservatively - the less, the better.

Tomorrow I'll experiment with using this new interface on an actual Oyster test... For now, let me know what you think!

btw: feel free to review the code, but don't expect much - it's quite late now, and I still need to review it myself before making it ready for review

[khamusa]

It's working as expected:

https://github.com/oysterhr/anywhere/pull/25458/commits/64a21a8a0b3a7167535d9195a14b17149d83b215
https://github.com/oysterhr/anywhere/pull/25458/commits/4cb86844d61e13af81fb7e88bf4906b773d147b7

[khamusa]

This PR is being superseded by #22