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

Jump to bottom

59 add localization comments to the dashboardview #250

Merged
merged 6 commits into from
Nov 4, 2023
Merged

59 add localization comments to the dashboardview #250

merged 6 commits into from
Nov 4, 2023

Conversation

hartti
Copy link
Contributor

@hartti hartti commented Oct 30, 2023

What it Does

How I Tested

  • Build the application
  • Localizable file should show all the new string comments

Notes

  • There are still strings without comments from other Views

Screenshot

Screenshot 2023-10-29 at 23 23 23

@hartti
Added localization comments for strings inside Text-elements. Did not…
97086b8 
… touch Labels and Buttons with String-only initilaizers yet.
@hartti
Added localization comments to Labels and Buttons - changed the initi…
86a6bd0 
…alizers to use View-based Label instread of StringProtocol
mikaelacaron
mikaelacaron requested changes Oct 31, 2023
View reviewed changes
Copy link
Owner

@mikaelacaron mikaelacaron left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great work!! I have several comments that need resolved please. If you have any questions, you can comment directly on that thread, otherwise click the resolve button once you've addressed my comment. Once you fix everything, please click the re-review button

Also please solve the merge conflict, and fix the indentation for a few spots

Re-review button

Basic-Car-Maintenance/Shared/Dashboard/Views/DashboardView.swift Outdated Show resolved Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/DashboardView.swift Outdated Show resolved Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/DashboardView.swift Show resolved Hide resolved
Basic-Car-Maintenance/Shared/Localizable.xcstrings Outdated
@@ -135,7 +136,7 @@
}
},
"Add" : {
"comment" : "Label for submit button on form to add an entry",
"comment" : "Label for submit button on form to add an entry\nLabel for adding maintenance event on Dashboard view",
Copy link
Owner

Choose a reason for hiding this comment

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

Note that this one has a \n on the comment, which means that this key Add has two different string comments, which at the moment causes a conflict.

To fix this you need to either use the same comment for both, or make a new localized string key for Add and use your separate comment.

I think making a new one is the best solution

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Owner

@mikaelacaron mikaelacaron Nov 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This wasn't resolved, I fixed it

@hartti
Copy link
Contributor Author

hartti commented Nov 4, 2023 •
edited
Loading

Uh... where is the re-review button? Newbie is getting a little lost here.. Sorry.

@mikaelacaron
Copy link
Owner

Uh... where is the re-review button? Newbie is getting a little lost here.. Sorry.

It's at the top right of the page when you're on the conversation tab

@mikaelacaron mikaelacaron self-requested a review November 4, 2023 22:30
mikaelacaron
mikaelacaron approved these changes Nov 4, 2023
View reviewed changes
Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

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

Great work!! Just a few formatting comments that I'll fix

Basic-Car-Maintenance/Shared/Dashboard/Views/DashboardView.swift Outdated Show resolved Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/DashboardView.swift Outdated Show resolved Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/DashboardView.swift Outdated Show resolved Hide resolved
Basic-Car-Maintenance/Shared/Dashboard/Views/DashboardView.swift Outdated Show resolved Hide resolved
Basic-Car-Maintenance/Shared/Localizable.xcstrings Outdated
@@ -135,7 +136,7 @@
}
},
"Add" : {
"comment" : "Label for submit button on form to add an entry",
"comment" : "Label for submit button on form to add an entry\nLabel for adding maintenance event on Dashboard view",
Copy link
Owner

@mikaelacaron mikaelacaron Nov 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This wasn't resolved, I fixed it

@mikaelacaron mikaelacaron merged commit c10d01b into mikaelacaron:dev Nov 4, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikaelacaron mikaelacaron mikaelacaron approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FEATURE - Add Localization Comments to the DashboardView
2 participants
@hartti @mikaelacaron