Skip to content

Commit

Permalink
Merge branch 'main' into fix-checkboxes
Browse files Browse the repository at this point in the history
  • Loading branch information
joelhawksley authored Oct 15, 2024
2 parents 0c29168 + cc1ce7a commit 65eee64
Show file tree
Hide file tree
Showing 26 changed files with 1,150 additions and 956 deletions.
5 changes: 5 additions & 0 deletions .changeset/heavy-camels-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

Fix problem introduced by #3141 that causes template captions not to render for certain input types if the input is configured with a `value:`.
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,4 @@ jobs:
if: steps.changed-files.outputs.any_changed == 'true'
run: |
npm ci
npx stylelint ${{ steps.changed-files.outputs.all_changed_files }} --custom-formatter=stylelint-actions-formatters
npx stylelint --rd -q ${{ steps.changed-files.outputs.all_changed_files }} --custom-formatter=stylelint-actions-formatters
12 changes: 12 additions & 0 deletions .stylelintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@
{
"ignoreAtRules": ["mixin", "define-mixin"]
}
],
"plugin/browser-compat": [
true,
{
"severity": "warning",
"allow": {
"features": ["properties.scrollbar-width"],
"flagged": false,
"partialImplementation": true,
"prefix": true
}
}
]
}
}
9 changes: 6 additions & 3 deletions app/components/primer/alpha/action_list.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ nav-list {
width: 100%;
height: 1px;
content: '';
background: var(--borderColor-muted, var(--color-action-list-item-inline-divider));
/* stylelint-disable-next-line primer/colors */
background: var(--borderColor-muted);
}

/* if descriptionWrap--inline exists, move pseudo divider to wrapper */
Expand All @@ -56,7 +57,8 @@ nav-list {
width: 100%;
height: var(--borderWidth-thin);
content: '';
background: var(--borderColor-muted, var(--color-action-list-item-inline-divider));
/* stylelint-disable-next-line primer/colors */
background: var(--borderColor-muted);
}

/* unset the default label pseudo */
Expand Down Expand Up @@ -673,7 +675,8 @@ nav-list {
margin-block-end: var(--base-size-8);
margin-inline: calc(-1 * var(--base-size-8));
list-style: none;
background: var(--borderColor-muted, var(--color-action-list-item-inline-divider));
/* stylelint-disable-next-line primer/colors */
background: var(--borderColor-muted);
border: 0;
}

Expand Down
1 change: 1 addition & 0 deletions app/components/primer/alpha/button_marketing.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
white-space: nowrap;
vertical-align: middle;
user-select: none;
/* stylelint-disable-next-line primer/colors */
background: linear-gradient(180deg, rgba(255, 255, 255, 0.15) 0%, rgba(255, 255, 255, 0) 100%),
var(--color-mktg-btn-bg) !important;
border: 0;
Expand Down
1 change: 0 additions & 1 deletion app/components/primer/alpha/dialog.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

/* TODO: One day this can be :has(:modal), when it is better supported */
body.has-modal {
/* stylelint-disable-next-line csstools/value-no-unknown-custom-properties */
padding-right: var(--dialog-scrollgutter) !important;
overflow: hidden !important;
}
Expand Down
5 changes: 4 additions & 1 deletion app/components/primer/alpha/dropdown.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
&::after {
/* stylelint-disable-next-line primer/borders */
border: 7px solid transparent;
/* stylelint-disable-next-line primer/colors */
border-bottom-color: var(--overlay-bgColor);
}

Expand Down Expand Up @@ -167,6 +168,7 @@
right: -14px;
left: auto;
border-color: transparent;
/* stylelint-disable-next-line primer/colors */
border-left-color: var(--overlay-bgColor);
}
}
Expand All @@ -192,6 +194,7 @@
/* stylelint-disable-next-line primer/spacing */
left: -14px;
border-color: transparent;
/* stylelint-disable-next-line primer/colors */
border-right-color: var(--overlay-bgColor);
}
}
Expand Down Expand Up @@ -226,7 +229,7 @@
bottom: -7px;
/* stylelint-disable-next-line primer/spacing */
left: 10px;
/* stylelint-disable-next-line primer/borders */
/* stylelint-disable-next-line primer/borders, primer/colors */
border-top: 7px solid var(--overlay-bgColor);
/* stylelint-disable-next-line primer/borders */
border-right: 7px solid transparent;
Expand Down
1 change: 1 addition & 0 deletions app/components/primer/alpha/layout.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@
width: 1px;
/* stylelint-disable-next-line primer/spacing */
margin-right: -1px;
/* stylelint-disable-next-line primer/colors */
background: var(--borderColor-default);
}

Expand Down
1 change: 1 addition & 0 deletions app/components/primer/alpha/menu.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
left: 0;
width: 2px;
content: '';
/* stylelint-disable-next-line primer/colors */
background-color: var(--underlineNav-borderColor-active);
}
}
Expand Down
1 change: 0 additions & 1 deletion app/components/primer/alpha/segmented_control.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@
/* stylelint-disable-next-line primer/spacing */
padding-inline: calc(var(--control-medium-paddingInline-normal) - var(--segmentedControl-item-padding));

/* stylelint-disable-next-line plugin/no-unsupported-browser-features */
&:focus-visible {
/* stylelint-disable-next-line primer/borders */
border-radius: calc(var(--borderRadius-medium) - var(--segmentedControl-item-padding) / 1);
Expand Down
8 changes: 6 additions & 2 deletions app/components/primer/alpha/text_field.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@
width: var(--borderWidth-thin);
height: var(--base-size-16);
content: '';
/* stylelint-disable-next-line primer/colors */
background: var(--borderColor-default);
}
}
Expand Down Expand Up @@ -598,6 +599,7 @@ input[type='checkbox'].FormControl-checkbox {
height: var(--base-size-16);
visibility: hidden;
content: '';
/* stylelint-disable-next-line primer/colors */
background-color: var(--control-checked-fgColor-rest);
transition: visibility 0s linear 230ms;
clip-path: inset(var(--base-size-16) 0 0 0);
Expand Down Expand Up @@ -637,18 +639,20 @@ input[type='checkbox'].FormControl-checkbox {

&:disabled {
cursor: not-allowed;
/* stylelint-disable-next-line primer/colors */
background-color: var(--control-fgColor-disabled);
/* stylelint-disable-next-line primer/colors */
border-color: var(--control-fgColor-disabled);
opacity: 1;

&::before {
/* stylelint-disable-next-line primer/colors */
background-color: var(--control-checked-fgColor-rest);
}
}

/* Windows High Contrast mode */
@media (forced-colors: active) {
/* stylelint-disable-next-line primer/colors */
background-color: canvastext;
border-color: canvastext;
}
Expand Down Expand Up @@ -700,6 +704,7 @@ input[type='radio'].FormControl-radio {

&[disabled], &:disabled {
cursor: not-allowed;
/* stylelint-disable-next-line primer/colors */
border-color: var(--control-fgColor-disabled);

& ~ .FormControl-radio-labelWrap {
Expand All @@ -717,7 +722,6 @@ input[type='radio'].FormControl-radio {

/* Windows High Contrast mode */
@media (forced-colors: active) {
/* stylelint-disable-next-line primer/colors */
background-color: canvastext;
border-color: canvastext;
}
Expand Down
1 change: 1 addition & 0 deletions app/components/primer/alpha/underline_nav.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
width: 100%;
height: 2px;
content: '';
/* stylelint-disable-next-line primer/colors */
background: var(--underlineNav-borderColor-active);
border-radius: var(--borderRadius-medium);
transform: translate(50%, -50%);
Expand Down
2 changes: 2 additions & 0 deletions app/components/primer/beta/avatar_stack.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
/* stylelint-disable-next-line primer/spacing */
margin-right: -11px;
background-color: var(--bgColor-default);
/* stylelint-disable-next-line primer/colors */
border-right: var(--borderWidth-thin) solid var(--bgColor-default);
border-radius: var(--borderRadius-small);
box-shadow: none;
Expand Down Expand Up @@ -141,6 +142,7 @@
/* stylelint-disable-next-line primer/spacing */
margin-left: -11px;
border-right: 0;
/* stylelint-disable-next-line primer/colors */
border-left: var(--borderWidth-thin) solid var(--bgColor-default);
}
}
1 change: 0 additions & 1 deletion app/components/primer/beta/blankslate.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@

/* At the time these styles were written,
`34rem` was our "small" breakpoint width */
/* stylelint-disable-next-line plugin/no-unsupported-browser-features */
@container (max-width: 34rem) {
.blankslate {
--blankslate-outer-padding-block: var(--base-size-20);
Expand Down
4 changes: 4 additions & 0 deletions app/components/primer/beta/popover.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
margin-left: -8px;
/* stylelint-disable-next-line primer/borders */
border: 7px solid transparent;
/* stylelint-disable-next-line primer/colors */
border-bottom-color: var(--overlay-bgColor);
}
}
Expand Down Expand Up @@ -70,6 +71,7 @@

&::after {
bottom: -14px;
/* stylelint-disable-next-line primer/colors */
border-top-color: var(--overlay-bgColor);
}
}
Expand Down Expand Up @@ -147,6 +149,7 @@

&::after {
right: -14px;
/* stylelint-disable-next-line primer/colors */
border-left-color: var(--overlay-bgColor);
}
}
Expand All @@ -162,6 +165,7 @@

&::after {
left: -14px;
/* stylelint-disable-next-line primer/colors */
border-right-color: var(--overlay-bgColor);
}
}
Expand Down
2 changes: 2 additions & 0 deletions app/components/primer/beta/timeline_item.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
display: block;
width: var(--borderWidth-thick);
content: '';
/* stylelint-disable-next-line primer/colors */
background-color: var(--borderColor-muted);
}

Expand All @@ -40,6 +41,7 @@
color: var(--fgColor-muted);
align-items: center;
background-color: var(--timelineBadge-bgColor);
/* stylelint-disable-next-line primer/colors */
border: var(--borderWidth-thick) solid var(--bgColor-default);
border-radius: 50%;
justify-content: center;
Expand Down
6 changes: 6 additions & 0 deletions app/forms/caption_template_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,11 @@ class CaptionTemplateForm < ApplicationForm
age_radios.radio_button(value: "young", label: "10-15")
age_radios.radio_button(value: "middle_aged", label: "16-21")
end

name_form.check_box_group(name: "places", label: "Cool places") do |check_group|
check_group.check_box(value: "lopez", label: "Lopez Island")
check_group.check_box(value: "bellevue", label: "Bellevue")
check_group.check_box(value: "seattle", label: "Seattle")
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<span>Bellevue caption</span>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<span>Lopez caption</span>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<span>Seattle caption</span>
12 changes: 4 additions & 8 deletions app/lib/primer/forms/dsl/check_box_input.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,10 @@ def supports_validation?
false
end

private

def caption_template_name
@caption_template_name ||= if @scheme == :array
:"#{name}_#{value}"
else
name.to_sym
end
def values_disambiguate_template_names?
# Check boxes submitted as an array all have the same name, so we return true here
# to ensure different caption templates can be attached to individual check box inputs.
@scheme == :array
end
end
end
Expand Down
10 changes: 9 additions & 1 deletion app/lib/primer/forms/dsl/input.rb
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,14 @@ def validation_error_icon_target
""
end

# Whether or not the `value:` argument should be used to determine the caption template
# for a given field. This is useful in especially radio button groups where each option
# has the same name but a different value. Check box groups where the values are submitted
# as an array also use this feature, since each check box also has the same name.
def values_disambiguate_template_names?
false
end

private

def input_data
Expand All @@ -309,7 +317,7 @@ def input_data
def caption_template_name
return nil unless name

@caption_template_name ||= if respond_to?(:value) && value.present?
@caption_template_name ||= if values_disambiguate_template_names? && respond_to?(:value) && value.present?
:"#{name}_#{value}"
else
name.to_sym
Expand Down
4 changes: 4 additions & 0 deletions app/lib/primer/forms/dsl/radio_button_input.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ def type
def supports_validation?
false
end

def values_disambiguate_template_names?
true
end
end
end
end
Expand Down
Loading

0 comments on commit 65eee64

Please sign in to comment.