[DataTable] Hide arrow padding when not sorting#51667
[DataTable] Hide arrow padding when not sorting#51667dkwingsmt merged 6 commits intoflutter:masterfrom
Conversation
fluttergithubbot
added
f: material design
dkwingsmt
changed the title
| /// some text. | ||
| /// | ||
| /// By default, this widget will only occupy the minimal space. If you want | ||
| /// it to take the entire remaining space, e.g. when you want to use [Center], |
There was a problem hiding this comment.
Would using an Expanded be the only way to center the label? Would it be possible to set a size of space to take up?
There was a problem hiding this comment.
Sorry if I misunderstood your question. The problem here is the widget structure, which is roughly:
TableCell
|- Row
|- label
|- arrow & padding
the default behavior is to align everything left, including the label and the arrow (right if numerical), and the only thing the developer can control is label.
We can't provide an Expanded by default since it changes the behavior to aligning label left and arrow right.
We might actually have to provide a new parameter alignment for full flexibility, but since it'll be a new feature & parameter, I'd rather not do in this PR.
| )), | ||
| )); | ||
|
|
||
| { |
There was a problem hiding this comment.
What are these brackets enclosing? Here and below.
There was a problem hiding this comment.
These brackets are only to scope nameText and nameCell between subtests so that I don't have to either use nameText1 nameText2 etc, or declare 5 empty variables at the start of the test.
| expect(find.descendant(of: nameCell, matching: find.byType(Icon)), findsOneWidget); | ||
| } | ||
|
|
||
| await tester.pumpWidget(MaterialApp( |
There was a problem hiding this comment.
This last pumpWidget and the following expectations looks like the first one, why do we check it again?
There was a problem hiding this comment.
It was a suggestion by Hixie that we test turning it on then off again just to make sure. I added comment to explain the intention, hopefully it's clearer.
| expect(tester.renderObject(find.text('column2')).attached, true); | ||
|
|
||
| // Wait for the tooltip timer | ||
| await tester.pumpAndSettle(const Duration(seconds: 1)); |
There was a problem hiding this comment.
Do you want to check something after this pumpAndSettle?
There was a problem hiding this comment.
Not really. This pumpAndSettle is mostly to prevent an exception thrown by Tooltip when its timer expires and schedules a new frame when the view has been destroyed. I've updated the comment to make it clearer.
|
@Piinks I've addressed the issues you brought up, can you take a look again? Thanks! |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
4 participants
Description
This PR fixes the problem that the arrow padding still displays when
DataColumn.onSortis null, thus allowing the label to be truly centered.This continues the work of #43735 by @andermoran.
Related Issues
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.