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

503 $active_module_element method for TealAppDriver #1156

Conversation

m7pr
Copy link
Contributor

@m7pr m7pr commented Mar 15, 2024

I found it handy to have a function like this in here #1127 (comment)

Proposing a method to exist at TealAppDriver directly.

@m7pr m7pr added the core label Mar 15, 2024
@m7pr m7pr requested review from averissimo and vedhav March 15, 2024 12:07
@m7pr m7pr changed the base branch from main to 503-introduce-shinytest2@main March 15, 2024 12:08
Copy link
Contributor

@vedhav vedhav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏽 Agreed, It would help when referencing modules inside the teal_module.

@m7pr m7pr merged commit 1478763 into 503-introduce-shinytest2@main Mar 15, 2024
1 check passed
@m7pr m7pr deleted the active_module_element@503-introduce-shinytest2@main branch March 15, 2024 13:27
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2024
@m7pr
Copy link
Contributor Author

m7pr commented Mar 15, 2024

@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
Copy link
Contributor Author

m7pr commented Mar 18, 2024

CC @vedhav

@vedhav
Copy link
Contributor

vedhav commented Mar 18, 2024

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
Copy link
Contributor Author

m7pr commented Mar 18, 2024

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
Copy link
Contributor Author

m7pr commented Mar 18, 2024

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
Copy link
Contributor

vedhav commented Mar 18, 2024

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants