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

Legend Strings Cleanup #3566

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented Jul 8, 2024

I am suggesting some changes by removing some words that seem redundant to me.
Please let me know if you think this is not clear in English.

Before After
Screenshot_20240708-113759 Screenshot_20240708-120111

@Navid200
Copy link
Collaborator Author

Navid200 commented Jul 8, 2024

The word color is currently used on this page.
My problem with that concept is that we are talking about the legend page. Color is what a legend is about. There is nothing other than color.

Of course, we can some day give options to the user to change the dot to look like a plus sign or a circle or a star ...
Then, legend would be about color as well as marker shape.
But, we don't have that option now in xDrip. Stating color so many times on this page seems redundant to me. So, I have removed the word color in an instance in this PR.

@Navid200
Copy link
Collaborator Author

Navid200 commented Jul 8, 2024

When a heading says background, I expect everything in that section to be about background, and it is.
So, there should be no need to state background for every item in that section. So, I have removed those.

The word Glucose is used on this page. xDrip is mainly about glucose.
We do show heartbeat. I appreciate that. But, that is what needs clarification, not glucose. I would state heartbeat for a dot or text that is not glucose. But, I wouldn't include glucose for a dot or text that is about glucose because that is almost everything.
I have removed glucose from a few instances.

@jamorham
Copy link
Collaborator

I think this is probably okay but it will trigger a lot of new translations

@Navid200
Copy link
Collaborator Author

Would it become acceptable if I break this into 2 PRs and you can merge them in sequence instead of in one shot?
Or, would it be acceptable if I break it into 3 PRs?
Or, what do you suggest please?

@jamorham
Copy link
Collaborator

Its not the size of the PR it is the work we are asking others to do so we have to be sure we're not going to change our minds and alter it all again shortly afterwards.

@Navid200
Copy link
Collaborator Author

Now, I understand and agree.

Just to let you know, we have a developer working on this: #3565
What you see in that discussion is all I know about the status.

After I had that conversation, I realized that sooner or later, we may have one more item added to that menu. I have to say that menu looks too crowded to me. That was when I decided to clean it up.

We can forget about this PR for now and wait for that work to complete. When/if that work is done, we can then decide how to deal with the menu including the new item.

I agree 100%; creating unnecessary work for volunteers is unacceptable.

Regardless, I am going to look at this PR now very carefully and review everything several times. I will also imagine that the new item has been added when I do the review and see if we need to make any changes to avoid any more changes needed later.

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.

2 participants