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

Refactor admin search forms #4277

Merged
merged 11 commits into from
Dec 7, 2020
Merged

Refactor admin search forms #4277

merged 11 commits into from
Dec 7, 2020

Conversation

javierm
Copy link
Member

@javierm javierm commented Dec 3, 2020 •
edited

Objectives

  • Make sure the search terms are kept in the form after a search
  • Improve search form accessibility for screen reader users
  • Make it easier to create new search forms
  • Reduce code duplication

@javierm javierm self-assigned this Dec 3, 2020
@javierm javierm added this to Reviewing in Consul Democracy via automation Dec 3, 2020
@javierm javierm force-pushed the admin_search branch 3 times, most recently from bc3f359 to 2f6f89d Compare December 4, 2020 12:16
taitus and others added 10 commits December 4, 2020 19:57
@taitus @javierm
Include the searched term in the search input form
7b62941
@javierm
Extract users search view to a component
2cf49b2 
We're going to make this search component more generic, but for now,
we're keeping the exact same behavior we had.
@javierm
Use form_tag in search form
9a24a8e 
This form does not depend on an object at all, so we can use a generic
tag which is model-independent.
@javierm
Use a generic name for the search parameter
155da08 
This way we can use it for any model.
@javierm
Add ARIA role and ARIA label to admin search form
b453ed6 
This way screen reader users will have an easier access to it.
@javierm
Use CSS to style the search component
49c22b8 
We simplify the view, and so make it easier to customize.
@javierm
Use a shared translation for search buttons
0a3acf3 
We were writing the same text over and over for the same translations.
Since they all serve the same function, it's perfectly fine for them to
have the same text, and so we can have a shared translation.
@javierm
Allow different labels in admin search component
9b07359
@javierm
Allow custom options in search form component
a9427f5 
So we keep the same API as the form_tag method.
@javierm
Reuse admin search component in other sections
e33794e 
There are some sections where we are not reusing it:

* The budget investments search is completely different, so this
  component isn't appropriate there
* Booth assignment and officers are slightly different, and I'm not
  entirely sure it's safe to refactor these cases
Consul Democracy automation moved this from Reviewing to Testing Dec 7, 2020
@taitus taitus self-assigned this Dec 7, 2020
@javierm javierm merged commit b778d65 into master Dec 7, 2020
Consul Democracy automation moved this from Testing to Release 1.3.0 Dec 7, 2020
@javierm javierm deleted the admin_search branch December 7, 2020 13:59
@javierm javierm mentioned this pull request Dec 22, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taitus taitus taitus approved these changes

Assignees

@taitus taitus

@javierm javierm

Labels
Refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@javierm @taitus