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

breaking(layout_column_wrap): Make width optional but must be named #853

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

gadenbuie
Copy link
Member

@gadenbuie gadenbuie commented Oct 19, 2023

Supersedes #799
Addresses posit-dev/py-shiny#732

Breaking: This PR makes width an optional argument and moves it to after the .... A code search shows that most people name the width argument, but in case they haven't we'll test to see if the first argument to layout_column_wrap() looks like a valid width value (a single numeric value or a valid CSS unit) and we'll emit a deprecation warning and use that first value as width.

Note: in the deprecation warning I'm assuming this breaking change will cause our next version to be v0.6.0 (in any case, we should review our deprecation warnings before the next release to make sure we have the right version in them).

Some code to try out this feature:

x <- lapply(1:3, \(x) card(lorem::ipsum(x)))

# `width` is no longer required
layout_column_wrap(!!!x)

# Explicit, named `width`
layout_column_wrap(width = 1/2, !!!x)
layout_column_wrap(width = "300px", !!!x)
layout_column_wrap(width = 400, !!!x)

# Implied as the first argument,
# leads to deprecation warning but gives same result
layout_column_wrap(1/2, !!!x)
layout_column_wrap("300px", !!!x)
layout_column_wrap(400, !!!x)

@gadenbuie gadenbuie self-assigned this Oct 19, 2023
@gadenbuie gadenbuie marked this pull request as ready for review October 19, 2023 19:18
@gadenbuie gadenbuie requested a review from cpsievert October 19, 2023 19:18
Copy link
Collaborator

@cpsievert cpsievert left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

I agree, the release should be tagged as v0.6.0.

After a quick look, I think we only need to update a deprecated_soft() call in value_box()?

@gadenbuie gadenbuie merged commit 3b0b564 into main Oct 19, 2023
12 checks passed
@gadenbuie gadenbuie deleted the breaking/layout-column-wrap-width branch October 19, 2023 20:25
schloerke added a commit to posit-dev/py-shiny that referenced this pull request Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants