Move settings arrays to resources #924

Merged
mmarif merged 6 commits from qwerty287/GitNex:array-resources into main 3 weeks ago

Describe what your pull request does and which issue you’re targeting

Closes #920


### Describe what your pull request does and which issue you’re targeting <!-- Create a new issue, if it doesn't exist yet --> Closes #920 <br><br> <!-- Make sure you are targeting the "main" branch, pull requests on release branches are only allowed for bug fixes. --> - [X] I carefully read the [contribution guidelines](https://codeberg.org/GitNex/GitNex/src/branch/main/CONTRIBUTING.md). - [X] I'm following the code standards as defined [here](https://codeberg.org/gitnex/GitNex/wiki/Code-Standards). - [X] By submitting this pull request, I permit GitNex to license my work under the [GNU General Public License v3](https://codeberg.org/GitNex/GitNex/src/branch/main/LICENSE).
qwerty287 added 1 commit 3 weeks ago
opyale added the
Improvement
label 3 weeks ago
Owner

Is this ready for review?

Is this ready for review?
mmarif added this to the 4.0.0 milestone 3 weeks ago
Poster

Yes!

Yes!
Poster

(And my other PR #923 too)

(And my other PR #923 too)
mmarif requested changes 3 weeks ago
String[] homeDefaultScreen_ = {getResources().getString(R.string.pageTitleMyRepos), getResources().getString(R.string.pageTitleStarredRepos), getResources().getString(R.string.pageTitleOrganizations),
getResources().getString(R.string.pageTitleRepositories), getResources().getString(R.string.pageTitleProfile), getResources().getString(R.string.pageTitleExplore),
getResources().getString(R.string.titleDrafts)};
String[] homeDefaultScreen_ = getResources().getStringArray(R.array.homeDefaultScreen);
mmarif commented 3 weeks ago
Poster
Owner

We should change this to more appropriate name like appDeepLinksDefaultScreen.

We should change this to more appropriate name like `appDeepLinksDefaultScreen`.
Poster

Why DeepLinks? That's the setting for the home screen that's opened if you open the app, right?

Why DeepLinks? That's the setting for the home screen that's opened if you open the app, right?
Poster

(Actually I forgot to move the DeepLinks to the XML file)

(Actually I forgot to move the DeepLinks to the XML file)
Poster

Done in 185de52f48

Done in https://codeberg.org/gitnex/GitNex/commit/185de52f4810df99fcaa0fea96442068e43ed088
qwerty287 marked this conversation as resolved
String[] homeDefaultScreenNew = {getResources().getString(R.string.pageTitleMyRepos), getResources().getString(R.string.pageTitleStarredRepos), getResources().getString(R.string.pageTitleOrganizations),
getResources().getString(R.string.pageTitleRepositories), getResources().getString(R.string.pageTitleProfile), getResources().getString(R.string.pageTitleExplore),
getResources().getString(R.string.titleDrafts), getResources().getString(R.string.pageTitleNotifications)};
String[] homeDefaultScreenNew = getResources().getStringArray(R.array.homeDefaultScreenNew);
mmarif commented 3 weeks ago
Poster
Owner

This to appDefaultHomeScreen

This to `appDefaultHomeScreen`
qwerty287 marked this conversation as resolved
<item>Pitch Black</item>
</string-array>
<string-array name="homeDefaultScreen">
mmarif commented 3 weeks ago
Poster
Owner

Same as above.

Same as above.
qwerty287 marked this conversation as resolved
<item>@string/titleDrafts</item>
</string-array>
<string-array name="homeDefaultScreenNew">
mmarif commented 3 weeks ago
Poster
Owner

Same as above

Same as above
qwerty287 marked this conversation as resolved
qwerty287 added 1 commit 3 weeks ago
qwerty287 added 1 commit 3 weeks ago
qwerty287 added 1 commit 3 weeks ago
mmarif requested changes 3 weeks ago
// link handler
String[] defaultScreen_ = {getResources().getString(R.string.generalDeepLinkSelectedText), getResources().getString(R.string.navRepos), getResources().getString(R.string.navOrg), getResources().getString(R.string.pageTitleNotifications), getResources().getString(R.string.navExplore)};
String[] defaultScreen_ = getResources().getStringArray(R.array.appDeepLinksDefaultScreen);
mmarif commented 3 weeks ago
Poster
Owner

Can you make the following changes to names:

defaultScreen_ to linkHandlerDefaultScreen

homeDefaultScreenNew to appHomeDefaultScreenNew

homeDefaultScreen_ to appHomeDefaultScreen

Can you make the following changes to names: `defaultScreen_` to `linkHandlerDefaultScreen` `homeDefaultScreenNew` to `appHomeDefaultScreenNew` `homeDefaultScreen_` to `appHomeDefaultScreen`
qwerty287 marked this conversation as resolved
qwerty287 added 1 commit 3 weeks ago
opyale requested changes 3 weeks ago
tinyDB.putInt("timeId", i);
if("Normal".equals(timeList[i])) {
if(getString(R.string.settingsDateTimeNormal).equals(timeList[i])) {
opyale commented 3 weeks ago
Poster
Collaborator

if(i == 1)

``if(i == 1)``
opyale commented 3 weeks ago
Poster
Collaborator

Maybe a switch statement would be even better here:

switch(i) {
	case 0:
		// store in db
		break;
	case 1:
		// store in db
		break;
}
Maybe a switch statement would be even better here: ```java switch(i) { case 0: // store in db break; case 1: // store in db break; } ```
Poster

Done in 6103e76e33
I also improved the link handler behavior, you could review it

Done in https://codeberg.org/gitnex/GitNex/commit/6103e76e33960d2df35bffb8d12233bfd3fda2d8 I also improved the link handler behavior, you could review it
qwerty287 marked this conversation as resolved
qwerty287 added 1 commit 3 weeks ago
opyale approved these changes 3 weeks ago
opyale left a comment

Nice!

qwerty287 requested review from mmarif 3 weeks ago
mmarif approved these changes 3 weeks ago
mmarif referenced this issue from a commit 3 weeks ago
mmarif merged commit b97fa45b18 into main 3 weeks ago

Reviewers

opyale approved these changes 3 weeks ago
mmarif approved these changes 3 weeks ago
The pull request has been merged as b97fa45b18.
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.