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

English strings.xml improvements. #1151

Closed
wants to merge 4 commits into from
Closed

English strings.xml improvements. #1151

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
717540c
English strings.xml improvements.
stephencmorton Jan 2, 2020
d744275
improve description of G4 Receiver barcode
tolot27 Jul 17, 2021
ddc58d5
change BG to Glucose in predictive_color
tolot27 Aug 3, 2021
6df9dc4
remove manufacturer trademark
tolot27 Feb 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@
<string name="title_activity_calibration_override">Calibration Override</string>
<string name="pref_header_cloud_storage">Cloud Storage</string>
<string name="auto_configure_title">Auto configure</string>
<string name="prefs_auto_config_summary">Auto configure using a barcode.</string>
<string name="scan_share2_barcode">Scan Share Barcode</string>
<string name="pref_share2_scan_barcode_summary">Or scan the barcode on the share receiver</string>
<string name="prefs_auto_config_summary">Auto configure using a QR code.</string>
tolot27 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

As previously mentioned I think that barcode is a better word because a QR code is a type of barcode and so it better represents the feature which can support QR or other barcode types. Changing it to QR code makes it more restrictive regarding what features can be included via the auto-configure scanning and I believe that the word barcode is widely understood to mean all types of barcodes, 1D, 2D etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jamorham I noticed that you merged #2022 which changes the icon to a QR code. Given that, I'm ok with keeping the text as "barcode".
At the very least, the juxtaposition clarifies to the users that WE consider the terms interchangeable. 👍

Explicitly calling it a 'barcode' scanner (with the old icon) was making people like myself assume that it explicitly didn't work with QR codes. This was especially confusing because it's only used for QR codes in the current state of XDrip.

<string name="scan_share2_barcode">Scan Share Receiver barcode</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

As we think this feature is likely obsolete I don't see much point in creating work for translators as I don't think clarification is needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In that case, we should remove this feature completely from xDrip. This will increase maintainability and also avoids work for translators of new languages like Estonian.

<string name="pref_share2_scan_barcode_summary">Or scan the barcode on the Share Receiver</string>

<!-- MongoDB Settings -->
<string name="pref_title_mongodb">MongoDB</string>
Expand Down Expand Up @@ -490,10 +490,10 @@
<string name="bad_glucose_values">Bad Glucose Values</string>
<string name="revert_to_default">Revert to Default</string>
<string name="filtered_values">Filtered Values</string>
<string name="treatment_color">Treatment Color</string>
<string name="treatment_color_dark">Treatment Color Dark</string>
<string name="predictive_color">Predictive Color</string>
<string name="predictive_color_dark">Predictive Color Dark</string>
<string name="treatment_color">Insulin On Board Color</string>
<string name="treatment_color_dark">Insulin Activity Color</string>
<string name="predictive_color">Glucose Prediction Color</string>
<string name="predictive_color_dark">COB Prediction Color</string>
tolot27 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

These probably could be changed if that would help users. I think if I'm expanding what appears on the graph in future then I would most likely add additional colors for that purpose. @Navid200 what do you think of these wording changes?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I find the word "Color" included at the end redundant as this is a legend and it is only a color legend and nothing else.
So, in documentation, I have the exact same words minus color at the end.
But, below the color legend, on the same page, I have added an image that let's the user also see the patterns for these 4 items. If you scroll down, you will see the graph.

If you look at the insulin activity curve, you can see that it is not easy to see. Only clarifying what the color is may not be enough to help a user what the curve is. Looking at the curve, it's not easy to associate it with the square of color in the legend. That's why I thought it's best to show a typical image.

So, I guess that's a long way of saying I don't love those wording changes.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, but "color" was in the strings before. It was in the strings before it is in the strings after. So you don't object to the change you'd like to make additional changes. You could choose to hold up this PR in order to get everything perfect, I see the benefit in that, although it seems to me that the discussions would likely go on for ever if that was the aim; or you could see if this change at least made things better and didn't make anything worse and realize that you have no objections and will likely create a further PR yourself in the future.

(Ok, this time I'm actually going to unsubscribe from this PR. It just makes me too frustrated.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Navid200 a reminder that incivility is not appropriate here.

In summary:

This PR started as changes suggested by @stephencmorton as to their personal opinion as to how to improve some parts of the user interface. It wasn't to solve any particular raised issue that I know of.

I rejected the graphical changes for aesthetic reasons. @tolot27 redid the QR code graphic with red highlight which I accepted.

I've stated my reasons for not wishing to change the generic term barcode because I feel it constrains what that menu can be used for and so limits future options. I believe the term barcode to be widely understood but if not we can continue to discuss that.

The only remaining parts to this PR is the wording for the predictive simulation colors which were originally left intentionally vague and with a view to future expansion. That time has probably passed and so it is reasonable to consider improving that wording to be better understood by users and I'm simply asking for other's opinion on this to come to a decision. I asked @Navid200 about this as they are likely far closer to understanding how users react to these settings with their experience providing support to users.

This is quite an involved process to change a few words, but I'm simply responding to repeated requests to examine this PR and user interfaces are important to get right. Normally a suggestion to make a change would arise from a specific need. I might be blind to the need to change this text as I of course already know what it means which is why I would ask others for their opinion. I don't think it is a particularly high priority but I'm willing to consider it if others do think its worthwhile.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would use these words:

Insulin on Board
Insulin Activity
Glucose Prediction
Carbs on Board

In addition, many don't know what "Flair Colors" mean.
So, I think it's a good idea to edit those too. But, that could be something else:
#2031

Copy link
Contributor

Choose a reason for hiding this comment

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

This is quite an involved process to change a few words, but I'm simply responding to repeated requests to examine this PR and user interfaces are important to get right. Normally a suggestion to make a change would arise from a specific need. I might be blind to the need to change this text as I of course already know what it means which is why I would ask others for their opinion. I don't think it is a particularly high priority but I'm willing to consider it if others do think its worthwhile.

@jamorham a completely understandable reaction and I doubt the OP would have bothered if he'd known it would ultimately explode into such a big discussion. I don't believe it's his (or anyone's) fault that it ended up consuming so much time though.

Are there changes to the process that can be implemented to reduce the overhead to the repo managers? As a new contributor, I've been instructed to keep my PRs as limited in scope/incremental as possible, but that does increase the per-change overhead somewhat. New contributors are also inherently going to start with more "trivial" changes, due to lack of familiarity with the code base.
When it comes to things like translation for example, how do we decide what's "too trivial" to change?

<string name="average_and_target_lines">Average and Target Lines</string>
<string name="eight_hour_average_line">8-Hour Average Line</string>
<string name="twenty_four_hour_average_line">24-Hour Average Line</string>
Expand Down