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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite the summary list (with slots all the way down 馃悽) #222

Merged
merged 4 commits into from
Jul 30, 2021

Conversation

peteryates
Copy link
Member

Summary list rows are too flexible for regular arguments, because:

  • key can be set with either text or HTML
  • value can be set with either text or HTML
  • there can be multiple actions
  • each action can have its text set with text or HTML
  • keys, values and actions all accept custom classes and HTML attributes

Multi-level slots are the only way to achieve the above requirements without making the arguments incredibly complicated.

Summary list rows are too flexible for regular arguments, because:

* key can be set with either text or HTML
* value can be set with either text or HTML
* there can be multiple actions
* each action can have its text set with text or HTML
* keys, values and actions all accept custom classes and HTML attributes

The only way to solve this without making it massively complicated is to
use slots all the way down.
Using Ruby's methods that end with a question mark in ternary statements
is ugly because of the two question marks a space apart that have
entirely different meanings:

some_condition? ? when_true : when false

Wrapping the condition in parens is the only elegant solution (other
than just constructing it differently to appease Rubocop).

(some_condition?) ? when_true : when false

Should probably be disabled by default
@peteryates peteryates force-pushed the version-2.0.0-pp-summary-list branch from 5ce3d25 to b422a63 Compare July 30, 2021 17:10
@peteryates peteryates merged commit be928a5 into version-2.0.0 Jul 30, 2021
@peteryates peteryates deleted the version-2.0.0-pp-summary-list branch July 30, 2021 20:06
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.

None yet

1 participant