feat: Add flexible layout for attribute editor #3123
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3123 +/- ##
========================================
Coverage 96.42% 96.42%
========================================
Files 787 789 +2
Lines 22195 22255 +60
Branches 7209 7636 +427
========================================
+ Hits 21401 21459 +58
- Misses 787 789 +2
Partials 7 7 ☔ View full report in Codecov by Sentry. |
gethinwebster
force-pushed
the
dev-v3-gethinw-attribute-editor
branch
3 times, most recently
from
9703af9 to
9717155
Compare
gethinwebster
force-pushed
the
dev-v3-gethinw-attribute-editor
branch
from
9717155 to
d628c43
Compare
gethinwebster
requested review from
avinashbot,
a team,
YueyingLu and
georgylobko
and removed request for
a team,
avinashbot and
YueyingLu
|
|
||
| interface ControlProps extends InputProps { | ||
| index: number; | ||
| setItems?: any; |
There was a problem hiding this comment.
[nitpick] why setItems is any? the input type for this function is defined as Tag[] below
| aria-labelledby={`${firstControlId}-label ${firstControlId}`} | ||
| > | ||
| {definition.map(({ info, label, constraintText, errorText, warningText, control }, defIndex) => { | ||
| ({ gridColumnStart, gridColumnEnd } = getItemGridColumns(layout, defIndex)); |
There was a problem hiding this comment.
Why gridColumnStart and gridColumnEnd are declared in closure?
There was a problem hiding this comment.
the final value of gridColumnEnd is needed for the remove button
gethinwebster
force-pushed
the
attribute-editor-pr
branch
from
b842818 to
afa4837
Compare
gethinwebster
force-pushed
the
dev-v3-gethinw-attribute-editor
branch
5 times, most recently
from
5b9e07c to
d9b0bb1
Compare
gethinwebster
force-pushed
the
dev-v3-gethinw-attribute-editor
branch
from
d9b0bb1 to
4a591b4
Compare
gethinwebster
force-pushed
the
dev-v3-gethinw-attribute-editor
branch
2 times, most recently
from
a82828f to
15ba8ac
Compare
gethinwebster
force-pushed
the
dev-v3-gethinw-attribute-editor
branch
from
15ba8ac to
ba48e9a
Compare
|
|
||
| useImperativeHandle(ref, () => ({ | ||
| focus: () => { | ||
| itemRef.current?.focus(); | ||
| buttonRef.current?.focus(); |
There was a problem hiding this comment.
Why do you need to declare a ref object for every type of the button? Wouldn't it be simpler to leave it as is?
There was a problem hiding this comment.
the types aren't compatible due to the 2 different focus methods on ButtonDropdownProps.Ref
This reverts commit 28012d5.
Description
Allow flexibility of "column" width in attribute editor.
Related links, issue #, if available: CClEAoSKVzvJ
(NB this is a 'dummy' PR against a branch that has some other related PRs already in (#3114, #3118) so that it can be reviewed in isolation but without merge conflicts later)
How has this been tested?
New dev pages and unit tests
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.