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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(ui): Adjust scroll from clipping children form fields in AlertDialog from showSourcesDialog (#1748) #1782

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

fix(ui): Adjust scroll from clipping children form fields in AlertDialog from showSourcesDialog (#1748) #1782

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
121 changes: 60 additions & 61 deletions lib/ui/views/settings/settingsFragment/settings_manage_sources.dart
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@ class SManageSources extends BaseViewModel {
return showDialog(
context: context,
builder: (context) => AlertDialog(
scrollable: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that the previous AlertDialog title was fixed in-place and the modified one is scrollable along the content with this line, as the title and content are wrapped with the scrollable.

It's not that big of an issue, it looks natural (even only noticed after attaching the docs & refs) but it's the cleanest way to fully address this issue on all devices.

🔨 Break the glass in case of concern:

If this is unwanted, the 'hacky' alternatives would be to add extra padding or size with a SizedBox or Padding having a magic number that could not work on some specific MediaQuery's, virtual keyboards, or screen sizes.

scroll-sample.mp4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be a plus because, after #1791, the title can now expand lines (grow in the Y axis [vertical] ) when it's too large to fit instead of clipping. This would make the [AlertDialog] content smaller and smaller (improbable but still possible), which at some point would be very hard to scroll and interact.

However, if the title and content are scrollable, this wouldn't be the case, as the user could easily scroll and focus on the content after a multi-line title.

title: Row(
children: <Widget>[
Text(t.settingsView.sourcesLabel),
Expand All @@ -38,75 +39,73 @@ class SManageSources extends BaseViewModel {
),
],
),
content: SingleChildScrollView(
child: Column(
children: <Widget>[
TextField(
controller: _orgPatSourceController,
autocorrect: false,
onChanged: (value) => notifyListeners(),
decoration: InputDecoration(
icon: Icon(
Icons.extension_outlined,
color: Theme.of(context).colorScheme.onSurfaceVariant,
),
border: const OutlineInputBorder(),
labelText: t.settingsView.orgPatchesLabel,
hintText: patchesRepo.split('/')[0],
content: Column(
children: <Widget>[
TextField(
controller: _orgPatSourceController,
autocorrect: false,
onChanged: (value) => notifyListeners(),
decoration: InputDecoration(
icon: Icon(
Icons.extension_outlined,
color: Theme.of(context).colorScheme.onSurfaceVariant,
),
border: const OutlineInputBorder(),
labelText: t.settingsView.orgPatchesLabel,
hintText: patchesRepo.split('/')[0],
),
const SizedBox(height: 8),
// Patches repository's name
TextField(
controller: _patSourceController,
autocorrect: false,
onChanged: (value) => notifyListeners(),
decoration: InputDecoration(
icon: const Icon(
Icons.extension_outlined,
color: Colors.transparent,
),
border: const OutlineInputBorder(),
labelText: t.settingsView.sourcesPatchesLabel,
hintText: patchesRepo.split('/')[1],
),
const SizedBox(height: 8),
// Patches repository's name
TextField(
controller: _patSourceController,
autocorrect: false,
onChanged: (value) => notifyListeners(),
decoration: InputDecoration(
icon: const Icon(
Icons.extension_outlined,
color: Colors.transparent,
),
border: const OutlineInputBorder(),
labelText: t.settingsView.sourcesPatchesLabel,
hintText: patchesRepo.split('/')[1],
),
const SizedBox(height: 8),
// Integrations owner's name
TextField(
controller: _orgIntSourceController,
autocorrect: false,
onChanged: (value) => notifyListeners(),
decoration: InputDecoration(
icon: Icon(
Icons.merge_outlined,
color: Theme.of(context).colorScheme.onSurfaceVariant,
),
border: const OutlineInputBorder(),
labelText: t.settingsView.orgIntegrationsLabel,
hintText: integrationsRepo.split('/')[0],
),
const SizedBox(height: 8),
// Integrations owner's name
TextField(
controller: _orgIntSourceController,
autocorrect: false,
onChanged: (value) => notifyListeners(),
decoration: InputDecoration(
icon: Icon(
Icons.merge_outlined,
color: Theme.of(context).colorScheme.onSurfaceVariant,
),
border: const OutlineInputBorder(),
labelText: t.settingsView.orgIntegrationsLabel,
hintText: integrationsRepo.split('/')[0],
),
const SizedBox(height: 8),
// Integrations repository's name
TextField(
controller: _intSourceController,
autocorrect: false,
onChanged: (value) => notifyListeners(),
decoration: InputDecoration(
icon: const Icon(
Icons.merge_outlined,
color: Colors.transparent,
),
border: const OutlineInputBorder(),
labelText: t.settingsView.sourcesIntegrationsLabel,
hintText: integrationsRepo.split('/')[1],
),
const SizedBox(height: 8),
// Integrations repository's name
TextField(
controller: _intSourceController,
autocorrect: false,
onChanged: (value) => notifyListeners(),
decoration: InputDecoration(
icon: const Icon(
Icons.merge_outlined,
color: Colors.transparent,
),
border: const OutlineInputBorder(),
labelText: t.settingsView.sourcesIntegrationsLabel,
hintText: integrationsRepo.split('/')[1],
),
const SizedBox(height: 20),
Text(t.settingsView.sourcesUpdateNote),
],
),
),
const SizedBox(height: 20),
Text(t.settingsView.sourcesUpdateNote),
],
),
actions: <Widget>[
TextButton(
Expand Down