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

MBS-5641: Show language/script in relevant reports #1157

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

reosarevok
Copy link
Member

@reosarevok reosarevok commented Jul 18, 2019

https://tickets.metabrainz.org/browse/MBS-5641

Mostly follows how it's done on release search results, but I chose to show "-" rather than nothing when language or script are missing, since I feel it makes what is missing more understandable.
Arguably could have been done so that the NoScript report only shows language and NoLanguage only script, but by showing both it also serves to show the editor whether some have already been fixed (since the language and script are loaded from the release entry and not from the saved report data).

<abbr title={l_languages(item.release.language.name)}>
{item.release.language.iso_code_3}
</abbr>
) : lp('?', 'unset_language_script')}
Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about using '[unset]' at first but it feels too long.

@reosarevok reosarevok added the QoL Non-urgent quality of life improvements label Sep 27, 2019
@reosarevok reosarevok force-pushed the MBS-5641 branch 2 times, most recently from 33d789d to 32373df Compare February 18, 2020 21:31
Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

LGTM, although you might want to update this PR to format columns as in #1420.

root/report/components/ReleaseList.js Outdated Show resolved Hide resolved
root/report/components/ReleaseList.js Outdated Show resolved Hide resolved
@reosarevok
Copy link
Member Author

I won't change it to match that because this is releases and for now we haven't changed release display. Maybe we should, eventually, but :)

@reosarevok reosarevok force-pushed the MBS-5641 branch 2 times, most recently from 74ce81f to 936611c Compare April 1, 2020 16:05
@reosarevok
Copy link
Member Author

Changed ? to - to match other tables where we display the fact we're missing data :)

Mostly follows how it's done on release search results, but I chose
to show "-" rather than nothing when language or script are missing,
since I feel it makes *what* is missing more understandable.
Arguably could have been done so that the NoScript report only shows
language and NoLanguage only script, but by showing both it also serves
to show the editor whether some have already been fixed (since the
language and script are loaded from the release entry and not from the
saved report data).
@reosarevok reosarevok merged commit 954762a into metabrainz:master Apr 1, 2020
@reosarevok reosarevok deleted the MBS-5641 branch April 1, 2020 16:55
mwiencek added a commit that referenced this pull request Apr 1, 2020
* master:
  Update POT files using the production database
  Skip exporting undefined or empty setlist to JSON
  Remove format_setlist filter since c7a6dc3
  MBS-10360: Whitelist User-Agent header in CORS preflight requests (#1439)
  MBS-7465: Update the tag cloud (#1440)
  MBS-5641: Show language/script in relevant reports (#1157)
  Strip BOM in remove_invalid_characters
  MBS-10730: Select DISTINCT video/standalone recordings (#1443)
  MBS-10724: Allow sorting artist lists by begin/end area
  MBS-10724: Allow sorting artist/place lists by dates
  MBS-10724: Allow sorting artist/place lists by area
  MBS-10724: Allow sorting RG lists by artist
  MBS-9728: Allow new emojis in titles
  Use addColonText when possible
  MBS-10680: Link to WS docs from the Details tab
  MBS-10679: Link to JSON WS from the Details tab
yvanzo added a commit that referenced this pull request Apr 13, 2020
* beta:
  Update translations from Transifex
  Update POT files using the production database
  Update translations from Transifex
  MBS-10640: Fix donation check (#1454)
  MBS-10753: Order artist collections by sort name (#1159)
  MBS-10735: Stop highlighting CD Baby links (#1446)
  MBS-10747: Fix misleading open edit message (#1452)
  MBS-10688: Show a human readable error when attaching a dupe CDTOC (#1438)
  MBS-10718: Always use link orders for grouping (#1434)
  MBS-9894: Fix timeline month off by 1 (#1450)
  Bump Flow to 0.122.0
  Update POT files using the production database
  Skip exporting undefined or empty setlist to JSON
  Remove format_setlist filter since c7a6dc3
  MBS-10360: Whitelist User-Agent header in CORS preflight requests (#1439)
  MBS-7465: Update the tag cloud (#1440)
  MBS-5641: Show language/script in relevant reports (#1157)
  Strip BOM in remove_invalid_characters
  MBS-10730: Select DISTINCT video/standalone recordings (#1443)
  MBS-10724: Allow sorting artist lists by begin/end area
  MBS-10724: Allow sorting artist/place lists by dates
  MBS-10724: Allow sorting artist/place lists by area
  MBS-10724: Allow sorting RG lists by artist
  MBS-9728: Allow new emojis in titles
  Use addColonText when possible
  MBS-10680: Link to WS docs from the Details tab
  MBS-10679: Link to JSON WS from the Details tab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QoL Non-urgent quality of life improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants