| Message ID | 20210722171251.2554142-4-raxel@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/5] static: add JS Cookie Library to get csrftoken for fetch requests | expand |
| Related | show |
Raxel Gutierrez <raxel@google.com> writes: > Added styling to the new patch list html code to make the change > property and bundle action forms more usable. Before [1] and after [2] > images for reference. > > [1] https://imgur.com/Pzelipp > [2] https://imgur.com/UtNJXuf Nice! I'll run it past the local Patchwork power users. There's a little 'jump to form' arrow (▾) that probably needs to point up now, rather than down. Kind regards, Daniel > > Signed-off-by: Raxel Gutierrez <raxel@google.com> > --- > htdocs/css/style.css | 77 ++++++++++++++++++++++++++++++++++++-------- > patchwork/forms.py | 44 +++++++++++++++++++------ > 2 files changed, 99 insertions(+), 22 deletions(-) > > diff --git a/htdocs/css/style.css b/htdocs/css/style.css > index 1bcc93e..9982f92 100644 > --- a/htdocs/css/style.css > +++ b/htdocs/css/style.css > @@ -1,3 +1,7 @@ > +:root { > + --light-color: #F7F7F7; > +} > + > h2 { > font-size: 25px; > margin: 18px 0 18px 0; > @@ -122,10 +126,6 @@ a.colinactive:hover { > div.filters { > } > > -div.patch-forms { > - margin-top: 1em; > -} > - > /* list order manipulation */ > > table.patchlist tr.draghover { > @@ -149,7 +149,7 @@ input#reorder-change { > .paginator { > text-align: right; > clear: both; > - margin: 8px 0 15px; > + margin: 8px 0 15px; > } > > .paginator .prev-na, > @@ -346,13 +346,62 @@ table.bundlelist td > padding-right: 2em; > } > > +.patch-list-actions { > + width: 100%; > + display: inline-flex; > + flex-wrap: wrap; > + justify-content: space-between; > +} > + > /* forms that appear for a patch */ > +.patch-forms { > + display: inline-flex; > + flex-wrap: wrap; > + margin: 16px 0px; > +} > + > div.patch-form { > - border: thin solid #080808; > - padding-left: 0.6em; > - padding-right: 0.6em; > - float: left; > - margin: 0.5em 5em 0.5em 10px; > + display: flex; > + flex-wrap: wrap; > + align-items: center; > +} > + > +select[class^=change-property-], .archive-patch-select, .add-bundle { > + padding: 4px; > + margin-right: 8px; > + box-sizing: border-box; > + border-radius: 4px; > + background-color: var(--light-color); > +} > + > +#patch-form-archive { > + display: flex; > + align-items: center; > + margin-right: 4px; > +} > + > +#patch-form-archive > label { > + margin: 0px; > +} > + > +#patch-form-archive > select, #patch-form-archive > input { > + margin: 0px 4px 0px 4px; > +} > + > +.patch-form-submit { > + font-weight: bold; > + padding: 4px; > +} > + > +#patch-form-bundle, #add-to-bundle, #remove-bundle { > + margin-left: 16px; > +} > + > +.create-bundle { > + padding: 4px; > + margin-right: 8px; > + box-sizing: border-box; > + border-radius: 4px; > } > > div.patch-form h3 { > @@ -371,15 +420,17 @@ div.patch-form ul { > margin-top: 0em; > } > > -/* forms */ > -table.form { > +.create-bundle { > + padding: 4px; > + margin-right: 8px; > + box-sizing: border-box; > + border-radius: 4px; > } > > span.help_text { > font-size: 80%; > } > > - > table.form td { > padding: 0.6em; > vertical-align: top; > diff --git a/patchwork/forms.py b/patchwork/forms.py > index 7f1fd31..9727932 100644 > --- a/patchwork/forms.py > +++ b/patchwork/forms.py > @@ -54,7 +54,10 @@ class BundleForm(forms.ModelForm): > field_mapping = {'name': 'bundle_name'} > name = forms.RegexField( > regex=r'^[^/]+$', min_length=1, max_length=50, required=False, > - error_messages={'invalid': 'Bundle names can\'t contain slashes'}) > + error_messages={'invalid': 'Bundle names can\'t contain slashes'}, > + widget=forms.TextInput( > + attrs={'class': 'create-bundle', > + 'placeholder': 'Bundle name'})) > > # Maps form fields 'name' attr rendered in HTML element > def add_prefix(self, field_name): > @@ -126,18 +129,28 @@ class PatchForm(forms.ModelForm): > def __init__(self, instance=None, project=None, *args, **kwargs): > super(PatchForm, self).__init__(instance=instance, *args, **kwargs) > self.fields['delegate'] = forms.ModelChoiceField( > - queryset=_get_delegate_qs(project, instance), required=False) > + queryset=_get_delegate_qs(project, instance), > + widget=forms.Select(attrs={'class': 'change-property-delegate'}), > + required=False) > > class Meta: > model = Patch > fields = ['state', 'archived', 'delegate'] > + widgets = { > + 'state': forms.Select( > + attrs={'class': 'change-property-state'}), > + 'archived': forms.CheckboxInput( > + attrs={'class': 'archive-patch-check'}), > + } > > > class OptionalModelChoiceField(forms.ModelChoiceField): > - no_change_choice = ('*', 'no change') > + no_change_choice = ('*', 'No change') > to_field_name = None > > - def __init__(self, *args, **kwargs): > + def __init__(self, *args, placeholder, className, **kwargs): > + self.no_change_choice = ('*', placeholder) > + self.widget = forms.Select(attrs={'class': className}) > super(OptionalModelChoiceField, self).__init__( > initial=self.no_change_choice[0], *args, **kwargs) > > @@ -166,6 +179,10 @@ class OptionalModelChoiceField(forms.ModelChoiceField): > > class OptionalBooleanField(forms.TypedChoiceField): > > + def __init__(self, className, *args, **kwargs): > + self.widget = forms.Select(attrs={'class': className}) > + super(OptionalBooleanField, self).__init__(*args, **kwargs) > + > def is_no_change(self, value): > return value == self.empty_value > > @@ -173,17 +190,26 @@ class OptionalBooleanField(forms.TypedChoiceField): > class MultiplePatchForm(forms.Form): > action = 'update' > archived = OptionalBooleanField( > - choices=[('*', 'no change'), ('True', 'Archived'), > - ('False', 'Unarchived')], > + className="archive-patch-select", > + choices=[('*', 'No change'), ('True', 'Archive'), > + ('False', 'Unarchive')], > coerce=lambda x: x == 'True', > - empty_value='*') > + empty_value='*', > + label="Archived") > > def __init__(self, project, *args, **kwargs): > super(MultiplePatchForm, self).__init__(*args, **kwargs) > self.fields['delegate'] = OptionalModelChoiceField( > - queryset=_get_delegate_qs(project=project), required=False) > + queryset=_get_delegate_qs(project=project), > + placeholder="Delegate to", > + className="change-property-delegate", > + label="Delegate to", > + required=False) > self.fields['state'] = OptionalModelChoiceField( > - queryset=State.objects.all()) > + queryset=State.objects.all(), > + placeholder="Change state", > + className="change-property-state", > + label="Change state") > > def save(self, instance, commit=True): > opts = instance.__class__._meta > -- > 2.32.0.432.gabb21c7263-goog > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
Raxel Gutierrez wrote: > Added styling to the new patch list html code to make the change > property and bundle action forms more usable. Before [1] and after [2] > images for reference. > > [1] https://imgur.com/Pzelipp > [2] https://imgur.com/UtNJXuf Ooh, this changes from Properties Change state: [no change v] Delegate to: [no change v] Archive: [no change v] [update] To [Change state v] [Delegate to v] Archived: [No change v] [update] Good use of vertical space, and this looks more intuitive to work with since it matches a more usual style for an action bar. > Signed-off-by: Raxel Gutierrez <raxel@google.com> > --- > htdocs/css/style.css | 77 ++++++++++++++++++++++++++++++++++++-------- I like the CSS changes --- this not only achieves some nice style changes, but also makes it a bit easier to read in the process and makes good use of flexboxes to handle reflow reasonably if the page is resized. [...] > --- a/patchwork/forms.py > +++ b/patchwork/forms.py The changes here look reasonable as well. So Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Thanks.
> There's a little 'jump to form' arrow (▾) that probably needs to point > up now, rather than down. My first impression of the arrow was to reach the bottom of the page since there's a whole list of patches and that's common behavior for other web apps. Now that you say it's meant to jump to the forms, it makes sense. However, I think it could now be removed as the patch list action bar will be visible at the beginning of the page. Do you agree that it could be removed, or having it point up is more ideal? Best, Raxel
diff --git a/htdocs/css/style.css b/htdocs/css/style.css index 1bcc93e..9982f92 100644 --- a/htdocs/css/style.css +++ b/htdocs/css/style.css @@ -1,3 +1,7 @@ +:root { + --light-color: #F7F7F7; +} + h2 { font-size: 25px; margin: 18px 0 18px 0; @@ -122,10 +126,6 @@ a.colinactive:hover { div.filters { } -div.patch-forms { - margin-top: 1em; -} - /* list order manipulation */ table.patchlist tr.draghover { @@ -149,7 +149,7 @@ input#reorder-change { .paginator { text-align: right; clear: both; - margin: 8px 0 15px; + margin: 8px 0 15px; } .paginator .prev-na, @@ -346,13 +346,62 @@ table.bundlelist td padding-right: 2em; } +.patch-list-actions { + width: 100%; + display: inline-flex; + flex-wrap: wrap; + justify-content: space-between; +} + /* forms that appear for a patch */ +.patch-forms { + display: inline-flex; + flex-wrap: wrap; + margin: 16px 0px; +} + div.patch-form { - border: thin solid #080808; - padding-left: 0.6em; - padding-right: 0.6em; - float: left; - margin: 0.5em 5em 0.5em 10px; + display: flex; + flex-wrap: wrap; + align-items: center; +} + +select[class^=change-property-], .archive-patch-select, .add-bundle { + padding: 4px; + margin-right: 8px; + box-sizing: border-box; + border-radius: 4px; + background-color: var(--light-color); +} + +#patch-form-archive { + display: flex; + align-items: center; + margin-right: 4px; +} + +#patch-form-archive > label { + margin: 0px; +} + +#patch-form-archive > select, #patch-form-archive > input { + margin: 0px 4px 0px 4px; +} + +.patch-form-submit { + font-weight: bold; + padding: 4px; +} + +#patch-form-bundle, #add-to-bundle, #remove-bundle { + margin-left: 16px; +} + +.create-bundle { + padding: 4px; + margin-right: 8px; + box-sizing: border-box; + border-radius: 4px; } div.patch-form h3 { @@ -371,15 +420,17 @@ div.patch-form ul { margin-top: 0em; } -/* forms */ -table.form { +.create-bundle { + padding: 4px; + margin-right: 8px; + box-sizing: border-box; + border-radius: 4px; } span.help_text { font-size: 80%; } - table.form td { padding: 0.6em; vertical-align: top; diff --git a/patchwork/forms.py b/patchwork/forms.py index 7f1fd31..9727932 100644 --- a/patchwork/forms.py +++ b/patchwork/forms.py @@ -54,7 +54,10 @@ class BundleForm(forms.ModelForm): field_mapping = {'name': 'bundle_name'} name = forms.RegexField( regex=r'^[^/]+$', min_length=1, max_length=50, required=False, - error_messages={'invalid': 'Bundle names can\'t contain slashes'}) + error_messages={'invalid': 'Bundle names can\'t contain slashes'}, + widget=forms.TextInput( + attrs={'class': 'create-bundle', + 'placeholder': 'Bundle name'})) # Maps form fields 'name' attr rendered in HTML element def add_prefix(self, field_name): @@ -126,18 +129,28 @@ class PatchForm(forms.ModelForm): def __init__(self, instance=None, project=None, *args, **kwargs): super(PatchForm, self).__init__(instance=instance, *args, **kwargs) self.fields['delegate'] = forms.ModelChoiceField( - queryset=_get_delegate_qs(project, instance), required=False) + queryset=_get_delegate_qs(project, instance), + widget=forms.Select(attrs={'class': 'change-property-delegate'}), + required=False) class Meta: model = Patch fields = ['state', 'archived', 'delegate'] + widgets = { + 'state': forms.Select( + attrs={'class': 'change-property-state'}), + 'archived': forms.CheckboxInput( + attrs={'class': 'archive-patch-check'}), + } class OptionalModelChoiceField(forms.ModelChoiceField): - no_change_choice = ('*', 'no change') + no_change_choice = ('*', 'No change') to_field_name = None - def __init__(self, *args, **kwargs): + def __init__(self, *args, placeholder, className, **kwargs): + self.no_change_choice = ('*', placeholder) + self.widget = forms.Select(attrs={'class': className}) super(OptionalModelChoiceField, self).__init__( initial=self.no_change_choice[0], *args, **kwargs) @@ -166,6 +179,10 @@ class OptionalModelChoiceField(forms.ModelChoiceField): class OptionalBooleanField(forms.TypedChoiceField): + def __init__(self, className, *args, **kwargs): + self.widget = forms.Select(attrs={'class': className}) + super(OptionalBooleanField, self).__init__(*args, **kwargs) + def is_no_change(self, value): return value == self.empty_value @@ -173,17 +190,26 @@ class OptionalBooleanField(forms.TypedChoiceField): class MultiplePatchForm(forms.Form): action = 'update' archived = OptionalBooleanField( - choices=[('*', 'no change'), ('True', 'Archived'), - ('False', 'Unarchived')], + className="archive-patch-select", + choices=[('*', 'No change'), ('True', 'Archive'), + ('False', 'Unarchive')], coerce=lambda x: x == 'True', - empty_value='*') + empty_value='*', + label="Archived") def __init__(self, project, *args, **kwargs): super(MultiplePatchForm, self).__init__(*args, **kwargs) self.fields['delegate'] = OptionalModelChoiceField( - queryset=_get_delegate_qs(project=project), required=False) + queryset=_get_delegate_qs(project=project), + placeholder="Delegate to", + className="change-property-delegate", + label="Delegate to", + required=False) self.fields['state'] = OptionalModelChoiceField( - queryset=State.objects.all()) + queryset=State.objects.all(), + placeholder="Change state", + className="change-property-state", + label="Change state") def save(self, instance, commit=True): opts = instance.__class__._meta
Added styling to the new patch list html code to make the change property and bundle action forms more usable. Before [1] and after [2] images for reference. [1] https://imgur.com/Pzelipp [2] https://imgur.com/UtNJXuf Signed-off-by: Raxel Gutierrez <raxel@google.com> --- htdocs/css/style.css | 77 ++++++++++++++++++++++++++++++++++++-------- patchwork/forms.py | 44 +++++++++++++++++++------ 2 files changed, 99 insertions(+), 22 deletions(-)