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.

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

860: Add term filter scope option. #936

Merged
merged 3 commits into from Oct 16, 2018
Merged

860: Add term filter scope option. #936

merged 3 commits into from Oct 16, 2018

Conversation

@toolstack
Copy link
Contributor

@toolstack toolstack commented Oct 4, 2018

Resolves #860.

@toolstack
Copy link
Contributor Author

@toolstack toolstack commented Oct 4, 2018

scope

Comments on layout welcome :)

@toolstack toolstack requested a review from ocean90 Oct 4, 2018
@toolstack toolstack self-assigned this Oct 4, 2018
@toolstack toolstack added this to the 3.0 milestone Oct 4, 2018
@garretthyder
Copy link

@garretthyder garretthyder commented Oct 4, 2018

Thanks @toolstack

The inputs and the term scope seem to be misaligned from their labels there.

And I wonder if it can be cleaned up a bit giving 'term' a dedicated column now that it has the input and the radio group. Something like this;
screen shot 2018-10-04 at 10 07 42 am

Or if Term should have precedence maybe;
screen shot 2018-10-04 at 10 08 25 am

And I noticed the box is displaced on your screen as it's wider and wonder if the bulk actions should have their own row so that the filter/sort/statuses are always on their own row and left aligned as sometimes like in your view the filter/sort boxes feel out of place when they appear in the middle of the page instead of left aligned.

@toolstack
Copy link
Contributor Author

@toolstack toolstack commented Oct 4, 2018

Yeah, the misalignment of the labels is a Firefox weirdness because the filters dropdown uses paragraph tags I think.

The box is displaced because it starts where ever the filter "button" is on the screen, so that's by design.

I'm not sure about splitting the rows, it makes the important stuff (the translations) appear even farther down the screen.

@toolstack
Copy link
Contributor Author

@toolstack toolstack commented Oct 4, 2018

How about something like this:

current filter

@toolstack
Copy link
Contributor Author

@toolstack toolstack commented Oct 4, 2018

Minor updates to the above suggestion:

Firefox:
current filter

Vivaldi:
chrome

@garretthyder
Copy link

@garretthyder garretthyder commented Oct 4, 2018

Thanks @toolstack I do like the update with Options as a label and the label being above avoids the misalignment on FF.

As to

The box is displaced because it starts where ever the filter "button" is on the screen, so that's by design.
I'm not sure about splitting the rows, it makes the important stuff (the translations) appear even farther down the screen.

I opened #942 to discuss this as I feel it's jarring when displaced like that and wouldn't push the translations much further down (maybe 20px).

@toolstack toolstack requested review from Mte90 and garretthyder Oct 15, 2018
@Mte90
Mte90 approved these changes Oct 15, 2018
Copy link
Contributor

@Mte90 Mte90 left a comment

Tested, deserve a mention on the blog that feature because help a lot on searching bad translation also for consitency as example.

@toolstack
Copy link
Contributor Author

@toolstack toolstack commented Oct 16, 2018

@Mte90 I'm dreading writing the change log and post for 3.0 ;)

I did mention it in today's post, I'm thinking for the 3.0 release we might have to do a few posts over the lead up to 3.0. Maybe one for "UI Improvements" like this, one for Variants, one for CLDR and a few others to cover off all the new stuff 😄.

@Mte90
Copy link
Contributor

@Mte90 Mte90 commented Oct 16, 2018

Good idea so we can promote all the changes!

@toolstack toolstack merged commit 717b22d into develop Oct 16, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@toolstack toolstack deleted the 860-term-filter-scope branch Oct 16, 2018
@toolstack toolstack restored the 860-term-filter-scope branch Oct 16, 2018
@pedro-mendonca
Copy link
Member

@pedro-mendonca pedro-mendonca commented Oct 16, 2018

Hi @toolstack
I know I’m probably late.
But why are these terms choices radio buttons instead of checkboxes?
It would allow a much more flexible selection of scope, instead of only the existent combinations.

Example: it’s not possible to choose simultaneously Originals and Context

@toolstack
Copy link
Contributor Author

@toolstack toolstack commented Oct 16, 2018

@pedro-mendonca primarily due to the complexity of the SQL statement, but it could be done.

@pedro-mendonca
Copy link
Member

@pedro-mendonca pedro-mendonca commented Oct 17, 2018

Hi @toolstack and @Mte90
I understand it might increase the complexity of the SQL statement, but it will give much more power to GlotPress search and usability.
Here is my late suggestion:

screenshot 2018-10-17 15 21 41

The layout is organized by priorities, of most used items, with most common items checked:

Main search fields

  • Term
  • User
  • Search button

Search scope

  • Originals - search in originals
  • Translations - search in translations
  • Contexts - search in contexts
  • Comments - search in comments
  • References - search in References

Status - filters which status to include in the search

  • Current
  • Waiting
  • Fuzzy
  • Untranslated
  • Obsolete
  • Rejected

Options

  • With comment - entries with comments
  • With context - entries with context
  • With warnings - entries with warnings
  • Case sensitive
@Mte90
Copy link
Contributor

@Mte90 Mte90 commented Oct 17, 2018

Maybe we can implement as it is for 3.0 release and in the next version improving also with gathering of feedbacks from the community so we can see what are the problems.
I am afraid that in networks like translate.wp.org can be very slow because the database is huge so is better to see that feature in action in this situation before do advanced changes.

@pedro-mendonca
Copy link
Member

@pedro-mendonca pedro-mendonca commented Oct 17, 2018

All the options above already exist in GP, it's just not possible to filter for some of them individualy.

@toolstack
Copy link
Contributor Author

@toolstack toolstack commented Oct 17, 2018

I think the idea of allowing a mix of search scope makes sense, but the suggested layout doesn't quite work, for example you can't search for a user in a context 😄

If you want to create a PR (and issue) for this change it might make it in to 3.0, but otherwise we can take a look at this for a future release.

@pedro-mendonca
Copy link
Member

@pedro-mendonca pedro-mendonca commented Oct 17, 2018

You're right, the Search scope applies only to Term input.

@pedro-mendonca
Copy link
Member

@pedro-mendonca pedro-mendonca commented Oct 17, 2018

@toolstack I've separated the user as some of the suggestions above, and opened the issue #955

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants