Skip to content

Conversation

MartinFournier
Copy link
Contributor

I cherry-picked @tomitrescak's commit from his PR (#133) so that it would be mergeable, thanks!

Ability to show the key in the UI and sort by the key or translation

Added a few things so that it can be sorted ascending or descending + insensitively.

@gadicc
Copy link
Owner

gadicc commented Jan 11, 2016

Hey, this is really great. Thanks for getting the ball rolling here again and for the extra features. A few things:

  1. I'm very against packages uses global template helpers that can leak into the app / other packages. In any case, there's no real need to have a strings() and sortedStrings() method... the original method already had some sort logic, I think all sort logic should simply be integrated into that original function to keep things nice and clean.
  2. I'd also suggest a note about clicking on the headings to change sort order, next to the other hints / options.
  3. While you're messing around on that line, would be great to add 44f6c92 for v2 :)

Some extras, not required :)

  1. Show an up or down arrow next to the appropriate heading for selected sort order. This probably negates the need for (2) above.
  2. Option for a 1st level sort order of untrans,fuzzy,trans.

Otherwise yeah I'd say the only real blocker is the global helper and this is some very welcome work!

@MartinFournier
Copy link
Contributor Author

@gadicc I skipped

Option for a 1st level sort order of untrans,fuzzy,trans.

But otherwise, I think this is complete.

gadicc added a commit that referenced this pull request Jan 11, 2016
@gadicc gadicc merged commit 270c020 into gadicc:v2 Jan 11, 2016
@gadicc
Copy link
Owner

gadicc commented Jan 11, 2016

Legend! Thanks for all the awesome work!

@MartinFournier MartinFournier deleted the v2-ui-key-ordering branch January 12, 2016 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants