Skip to content

Commit b8c2963

Browse files
matyasfmatyasf
committed
refactor(ui-form-field): most layout fun
1 parent e28cf4a commit b8c2963

File tree

4 files changed

+84
-44
lines changed

4 files changed

+84
-44
lines changed

‎packages/ui-form-field/src/FormFieldLayout/index.tsx‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import {
3434
import { withStyle, jsx } from '@instructure/emotion'
3535
import { FormFieldMessages } from '../FormFieldMessages'
3636
import generateStyle from './styles'
37-
import { propTypes, allowedProps } from './props'
37+
import { propTypes, allowedProps, FormFieldStyleProps } from './props'
3838
import type { FormFieldLayoutProps } from './props'
3939
import generateComponentTheme from './theme'
4040

@@ -79,19 +79,39 @@ class FormFieldLayout extends Component<FormFieldLayoutProps> {
7979
}
8080

8181
componentDidMount() {
82-
this.props.makeStyles?.()
82+
this.props.makeStyles?.(this.makeStyleProps())
8383
}
8484

8585
componentDidUpdate() {
86-
this.props.makeStyles?.()
86+
this.props.makeStyles?.(this.makeStyleProps())
87+
}
88+
89+
makeStyleProps = (): FormFieldStyleProps => {
90+
return {
91+
hasMessages: this.hasMessages,
92+
hasVisibleLabel: this.hasVisibleLabel
93+
}
8794
}
8895

8996
get hasVisibleLabel() {
90-
return this.props.label && hasVisibleChildren(this.props.label)
97+
return this.props.label ? hasVisibleChildren(this.props.label) : false
9198
}
9299

93100
get hasMessages() {
94-
return this.props.messages && this.props.messages.length > 0
101+
if (!this.props.messages || this.props.messages.length == 0) {
102+
return false
103+
}
104+
for (const msg of this.props.messages) {
105+
if (msg.text) {
106+
if (typeof msg.text === 'string') {
107+
return msg.text.length > 0
108+
}
109+
// this is more complicated (e.g. an array, a React component,...)
110+
// but we don't try to optimize here for these cases
111+
return true
112+
}
113+
}
114+
return false
95115
}
96116

97117
get elementType() {
@@ -107,21 +127,27 @@ class FormFieldLayout extends Component<FormFieldLayoutProps> {
107127
renderLabel() {
108128
if (this.hasVisibleLabel) {
109129
if (this.elementType == 'fieldset') {
110-
// `legend` has some very special buildt in CSS, this can only be reset
130+
// `legend` has some special built in CSS, this can only be reset
111131
// this way https://stackoverflow.com/a/65866981/319473
112132
return (
113133
<legend style={{ display: 'contents' }}>
114-
<div css={this.props.styles?.formFieldLabel}>
134+
<span css={this.props.styles?.formFieldLabel}>
115135
{this.props.label}
116-
</div>
136+
</span>
117137
</legend>
118138
)
119139
}
120140
return (
121141
<span css={this.props.styles?.formFieldLabel}>{this.props.label}</span>
122142
)
123143
} else if (this.props.label) {
124-
// TODO check this case
144+
if (this.elementType == 'fieldset') {
145+
return (
146+
<legend id={this._labelId} style={{ display: 'contents' }}>
147+
{this.props.label}
148+
</legend>
149+
)
150+
}
125151
// needs to be wrapped because it needs an `id`
126152
return (
127153
<div id={this._labelId} css={this.props.styles?.formFieldLabel}>

‎packages/ui-form-field/src/FormFieldLayout/props.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,13 @@ const allowedProps: AllowedPropKeys = [
131131
//'vAlign'
132132
]
133133

134+
type FormFieldStyleProps = {
135+
hasMessages: boolean
136+
hasVisibleLabel: boolean
137+
}
138+
134139
export type {
140+
FormFieldStyleProps,
135141
FormFieldLayoutProps,
136142
FormFieldLayoutStyle,
137143
FormFieldLayoutOwnProps

‎packages/ui-form-field/src/FormFieldLayout/styles.ts‎

Lines changed: 39 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,12 @@
2222
* SOFTWARE.
2323
*/
2424

25-
import type { FormFieldLayoutProps, FormFieldLayoutStyle } from './props'
25+
import type {
26+
FormFieldLayoutProps,
27+
FormFieldLayoutStyle,
28+
FormFieldStyleProps
29+
} from './props'
2630
import type { FormFieldLayoutTheme } from '@instructure/shared-types'
27-
import { hasVisibleChildren } from '@instructure/ui-a11y-utils'
2831
// TODO If the label is invisible the top gap is too large
2932
// TODO DateTimeInput renders empty messages, these leave too large gaps too
3033
/**
@@ -34,63 +37,62 @@ import { hasVisibleChildren } from '@instructure/ui-a11y-utils'
3437
* Generates the style object from the theme and provided additional information
3538
* @param {Object} componentTheme The theme variable object.
3639
* @param {Object} props the props of the component, the style is applied to
40+
* @param {Object} styleProps
3741
* @return {Object} The final style object, which will be used in the component
3842
*/
3943
const generateStyle = (
4044
componentTheme: FormFieldLayoutTheme,
41-
props: FormFieldLayoutProps
45+
props: FormFieldLayoutProps,
46+
styleProps: FormFieldStyleProps
4247
): FormFieldLayoutStyle => {
43-
const { inline, layout, label, messages } = props
44-
const hasLabelContent = hasVisibleChildren(label)
45-
const hasMessages = messages && messages.length > 0
48+
const { inline, layout } = props
49+
const { hasMessages, hasVisibleLabel } = styleProps
4650
// This is quite ugly, we should simplify it
4751
const inlineContainerAndLabel = layout === 'inline' && !inline
4852
const isInlineLayout = layout === 'inline'
4953
let gridTemplateColumns = '100%'
5054
// stacked layout
51-
let gridTemplateAreas = `"label"
55+
let gridTemplateAreas = `${hasVisibleLabel ? ' "label"' : ''}
5256
"controls"
5357
${hasMessages ? ' "messages"' : ''}`
5458
if (isInlineLayout) {
55-
gridTemplateColumns = `1fr 3fr`
56-
gridTemplateAreas = `"label controls"
59+
gridTemplateColumns = '1fr 3fr'
60+
gridTemplateAreas = `${hasVisibleLabel ? ' "label controls"' : '. controls'}
5761
${hasMessages ? '"messages messages"' : ''}`
5862
if (inline) {
5963
gridTemplateColumns = 'auto 3fr'
6064
}
6165
}
6266
if (inlineContainerAndLabel) {
63-
gridTemplateAreas = `"label controls"
67+
gridTemplateAreas = `${hasVisibleLabel ? ' "label controls"' : '. controls'}
6468
${hasMessages ? ' ". messages"' : ''}`
6569
}
6670

6771
const labelStyles = {
6872
all: 'initial',
6973
display: 'block',
7074
gridArea: 'label',
71-
...(hasLabelContent && {
72-
color: componentTheme.color,
73-
fontFamily: componentTheme.fontFamily,
74-
fontWeight: componentTheme.fontWeight,
75-
fontSize: componentTheme.fontSize,
76-
lineHeight: componentTheme.lineHeight,
77-
margin: 0,
78-
textAlign: inlineContainerAndLabel ? 'end' : 'start',
79-
...(isInlineLayout &&
80-
inline && {
81-
paddingRight: componentTheme.inlinePadding
82-
}),
83-
...(inlineContainerAndLabel && {
84-
paddingLeft: componentTheme.inlinePadding,
75+
color: componentTheme.color,
76+
fontFamily: componentTheme.fontFamily,
77+
fontWeight: componentTheme.fontWeight,
78+
fontSize: componentTheme.fontSize,
79+
lineHeight: componentTheme.lineHeight,
80+
margin: 0,
81+
textAlign: inlineContainerAndLabel ? 'end' : 'start',
82+
...(isInlineLayout &&
83+
inline && {
8584
paddingRight: componentTheme.inlinePadding
8685
}),
87-
[`@media screen and (max-width: ${componentTheme.stackedOrInlineBreakpoint})`]:
88-
{
89-
...(inlineContainerAndLabel && {
90-
textAlign: 'start'
91-
})
92-
}
93-
})
86+
...(inlineContainerAndLabel && {
87+
paddingLeft: componentTheme.inlinePadding,
88+
paddingRight: componentTheme.inlinePadding
89+
}),
90+
[`@media screen and (max-width: ${componentTheme.stackedOrInlineBreakpoint})`]:
91+
{
92+
...(inlineContainerAndLabel && {
93+
textAlign: 'start'
94+
})
95+
}
9496
}
9597

9698
return {
@@ -135,10 +137,12 @@ const generateStyle = (
135137
},
136138
formFieldLabel: {
137139
label: 'formFieldLayout__label',
138-
...labelStyles,
139-
// NOTE: needs separate groups for `:is()` and `:-webkit-any()` because of css selector group validation (see https://www.w3.org/TR/selectors-3/#grouping)
140-
'&:is(label)': labelStyles,
141-
'&:-webkit-any(label)': labelStyles
140+
...(hasVisibleLabel && {
141+
...labelStyles,
142+
// NOTE: needs separate groups for `:is()` and `:-webkit-any()` because of css selector group validation (see https://www.w3.org/TR/selectors-3/#grouping)
143+
'&:is(label)': labelStyles,
144+
'&:-webkit-any(label)': labelStyles
145+
})
142146
},
143147
formFieldChildren: {
144148
label: 'formFieldLayout__children',

‎packages/ui-form-field/src/FormPropTypes.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ type FormMessageType =
4040
| 'hint'
4141
| 'success'
4242
| 'screenreader-only'
43+
// TODO it will be easier if this would be just a string
44+
/**
45+
* The text to display in the form message
46+
*/
4347
type FormMessageChild = React.ReactNode
4448

4549
/**

0 commit comments

Comments
 (0)