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

Conversation

toolstack
Copy link
Contributor

Resolves #860.

@toolstack
Copy link
Contributor Author

scope

Comments on layout welcome :)

@toolstack toolstack requested a review from ocean90 October 4, 2018 13:26
@toolstack toolstack self-assigned this Oct 4, 2018
@toolstack toolstack added the [Type] Enhancement A suggestion for improvement. label Oct 4, 2018
@toolstack toolstack added this to the 3.0 milestone Oct 4, 2018
@garretthyder
Copy link

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

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

How about something like this:

current filter

@toolstack
Copy link
Contributor Author

toolstack commented Oct 4, 2018

Minor updates to the above suggestion:

Firefox:
current filter

Vivaldi:
chrome

@garretthyder
Copy link

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).

Copy link
Contributor

@Mte90 Mte90 left a comment

Choose a reason for hiding this comment

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

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

@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 commented Oct 16, 2018

Good idea so we can promote all the changes!

@toolstack toolstack merged commit 717b22d into develop Oct 16, 2018
@toolstack toolstack deleted the 860-term-filter-scope branch October 16, 2018 16:10
@toolstack toolstack restored the 860-term-filter-scope branch October 16, 2018 16:25
@pedro-mendonca
Copy link
Member

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

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

@pedro-mendonca
Copy link
Member

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 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

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

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

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

@pedro-mendonca
Copy link
Member

@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
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants