insightsengineering / teal

Exploratory Web Apps for Analyzing Clinical Trial Data
https://insightsengineering.github.io/teal/
Other
168 stars 33 forks source link

503 `$active_module_element` method for `TealAppDriver` #1156

Closed m7pr closed 4 months ago

m7pr commented 4 months ago

I found it handy to have a function like this in here https://github.com/insightsengineering/teal/pull/1127#issuecomment-1994283810

Proposing a method to exist at TealAppDriver directly.

m7pr commented 4 months ago

@vedhav what about one more method

active_module_element_text <- function(selector) {
    self$active_module_element(selector) %>% get_text()
}

So instead of

app$active_module_element("copy_button1") %>%
    app$get_text()

You just type

app$active_module_element_text("copy_button1")
m7pr commented 4 months ago

CC @vedhav

vedhav commented 4 months ago

Thanks for the ping. After Paweł bought it up. It makes me wonder if we have too many of these methods which makes it hard for people to know what they do. Let's rethink about trying to simplify the usage of these methods once the dust settles and we narrow down all the methods we need.

m7pr commented 4 months ago

I don't think Pawel mentioned we have too many methods. What he mentioned was we should just organize them so it's easy to use and seek trhough in an alphabetically ordered list. If we think a method is useful and makes the code shorter I think we should go for whathever the amount of methods that makes our life easier

m7pr commented 4 months ago

We can have $active_module_element_name or $active_module_element_id and $active_module_element_text. So one method gets the ID of the element, and the other one get's the text : )

vedhav commented 4 months ago

Yeah, let’s extend it to your suggestions. They’re definitely improvement. In the future we can think about some nomenclature like app$active$element_name