gitnex/GitNex
gitnex/GitNex
Archived
14
57
Fork 14
Code Issues Pull Requests 2 Releases 22 Wiki Activity

Theme and Icons #260

Merged
mmarif merged 15 commits from light-theme into master 2020-03-08 13:52:34 +00:00
Conversation 9 Commits 15 Files Changed 214 +1387 -948
mmarif commented 2020-01-29 14:50:51 +00:00
Owner
Copy Link

Will close #249 and #250

Will close #249 and #250
mmarif added this to the 2.4.0 milestone 2020-01-29 14:50:51 +00:00
mmarif added the
Feature
UI/UX
 labels 2020-01-29 14:50:51 +00:00
mmarif self-assigned this 2020-01-29 14:50:51 +00:00
mmarif changed title from [WIP] Theme and Icons to Theme and Icons 2020-03-06 13:47:38 +00:00
mmarif added the
LGTM-need
 label 2020-03-06 13:47:52 +00:00
6543 commented 2020-03-06 13:59:23 +00:00
Member
Copy Link

I'll have a look :)

I'll have a look :)
👍 1
6543 started working 2020-03-06 18:36:08 +00:00
6543 reviewed 2020-03-06 18:48:52 +00:00
Dismissed
app/src/main/java/org/mian/gitnex/activities/BaseActivity.java Outdated
@ -16,1 +17,4 @@
final TinyDB tinyDb = new TinyDB(getApplicationContext());
if(tinyDb.getInt("themeId") == 0) {
6543 commented 2020-03-06 18:48:52 +00:00
Member

can you test for == 1 and else set default theme?

can you test for == 1 and else set default theme?
mmarif commented 2020-03-06 20:04:12 +00:00
Author
Owner

Yes, that will work too.

Yes, that will work too.
6543 reviewed 2020-03-06 19:02:57 +00:00
Dismissed
6543 left a comment
Member

one nit, a question

one nit, a question
app/src/main/java/org/mian/gitnex/adapters/RepoStargazersAdapter.java Outdated
@ -113,6 +114,16 @@ public class RepoStargazersAdapter extends BaseAdapter {
viewHolder.memberName.setTypeface(myTypeface);
}
if(tinyDb.getInt("themeId") == 0) { // dark
6543 commented 2020-03-06 18:53:03 +00:00
Member

same as first comment - why not make if (is light) [else if (is other theme - not here jet)] else default -> dark theme

same as first comment - why not make if (is light) [else if (is other theme - not here jet)] else default -> dark theme
mmarif commented 2020-03-06 20:04:25 +00:00
Author
Owner

Yup, you are right.

Yup, you are right.
app/src/main/res/drawable/ic_android.xml
@ -1,4 +1,4 @@
<vector android:height="24dp" android:tint="#FFFFFF"
<vector android:height="24dp" android:tint="#368f73"
6543 commented 2020-03-06 19:01:58 +00:00
Member

would be nice if this would be a in the theme config too - but dont know how hard this is to implement ...

would be nice if this would be a in the theme config too - but dont know how hard this is to implement ...
mmarif commented 2020-03-06 20:06:28 +00:00
Author
Owner

Not sure how hard. Haven't researched on this. But for now the same green color icons matches both themes. Will see if we have more themes later on.

Not sure how hard. Haven't researched on this. But for now the same green color icons matches both themes. Will see if we have more themes later on.
mmarif commented 2020-03-07 07:48:45 +00:00
Author
Owner

Just tried with custom attr and it works. But for now we will keep it as we can improve the themes slowly. Also this PR intended job is done and is very big already.

Just tried with custom attr and it works. But for now we will keep it as we can improve the themes slowly. Also this PR intended job is done and is very big already.
6543 commented 2020-03-07 22:17:04 +00:00
Member

yes have same opinion but nice that is is doable - I'll create an issue witch links to this comments so we have a record of this :)

yes have same opinion but nice that is is doable - I'll create an issue witch links to this comments so we have a record of this :)
6543 commented 2020-03-07 22:18:57 +00:00
Member

-> #271

-> #271
6543 stopped working 2020-03-06 19:03:08 +00:00
27min
mmarif commented 2020-03-06 20:09:27 +00:00
Author
Owner
Copy Link

Ready for final review. :)

Ready for final review. :)
6543 commented 2020-03-06 21:17:19 +00:00
Member
Copy Link

Cant view diff with GitNex I'll have to wait tomoroww to look at the pull via PC

--> GitNex crash because of to big diff, can we ad something like paggination?

Cant view diff with GitNex I'll have to wait tomoroww to look at the pull via PC --> GitNex crash because of to big diff, can we ad something like paggination?
mmarif commented 2020-03-07 06:34:31 +00:00
Author
Owner
Copy Link

GitNex crash because of to big diff, can we ad something like paggination?

Currently there is no API for it so I am rendering it from web. I know it is not best at this time and may have problems. But with few files it works. With PR this big it was going to crash/stop work etc.

Room of improvement are very minimal in the current imeplementation tbh. I will leave it as is except fix bugs and wait for the API to be available at later times to have a better implementation of it.

> GitNex crash because of to big diff, can we ad something like paggination? Currently there is no API for it so I am rendering it from web. I know it is not best at this time and may have problems. But with few files it works. With PR this big it was going to crash/stop work etc. Room of improvement are very minimal in the current imeplementation tbh. I will leave it as is except fix bugs and wait for the API to be available at later times to have a better implementation of it.
6543 commented 2020-03-07 14:18:14 +00:00
Member
Copy Link

O didn't noticed there is an api issue, is there an issue open upstream?

O didn't noticed there is an api issue, is there an issue open upstream?
mmarif commented 2020-03-07 14:59:42 +00:00
Author
Owner
Copy Link

Haven't looked around but did not come across so far.

Haven't looked around but did not come across so far.
6543 started working 2020-03-07 22:08:02 +00:00
6543 reviewed 2020-03-07 22:22:06 +00:00
Dismissed
app/src/main/java/org/mian/gitnex/activities/FileViewActivity.java Outdated
@ -49,2 +50,4 @@
private PDFView pdfView;
private LinearLayout pdfViewFrame;
private byte[] decodedPdf;
private Boolean $nightMode;
6543 commented 2020-03-07 22:12:13 +00:00
Member

why this variable start with $ ?

why this variable start with `$` ?
mmarif commented 2020-03-08 05:50:59 +00:00
Author
Owner

Good catch. I was into PHP mode. :)

Good catch. I was into PHP mode. :)
6543 stopped working 2020-03-07 22:22:26 +00:00
14min 24s
6543 approved these changes 2020-03-08 13:32:34 +00:00
Dismissed
6543 commented 2020-03-08 13:41:53 +00:00
Member
Copy Link

php mode 😆 - I wonder it works!

I have tested it now - works as expected 👍

php mode :laughing: - I wonder it works! I have tested it now - works as expected :+1:
🎉 1
6543 added
LGTM-done
 and removed
LGTM-need
 labels 2020-03-08 13:42:08 +00:00
mmarif commented 2020-03-08 13:45:15 +00:00
Author
Owner
Copy Link

I wonder it works!

Haha, java treats it as a character like others.

> I wonder it works! Haha, java treats it as a character like others.
6543 commented 2020-03-08 13:51:48 +00:00
Member
Copy Link

I'm fine with merging this :D

I'm fine with merging this :D
mmarif closed this pull request 2020-03-08 13:52:34 +00:00
mmarif deleted branch light-theme 2020-03-08 13:52:44 +00:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
Labels
Clear labels   
Priority-high

   
Accepting-merge-requests

   
API

   
API-dependency

   
Backport

   
Blocked

   
Brainstorming

   
Breaking

Breaking change

  
Bug

   
Changelog

   
CI

   
Cleanup

   
Confirmed

   
Discussion

   
Documentation

   
Duplicate

   
Enhancement

   
External-dependecy

   
F-droid

   
Feature

   
Google-play

   
Improvement

   
Invalid

   
Investigate

   
LGTM-done

legitimation approved

  
LGTM-need

one approval required

  
Long-term

   
Major-release

   
Minor-release

   
Needs-cleanup

   
Needs-feedback

   
Needs-help

   
Priority-critical

   
Priority-low

   
Priority-medium

   
Question

   
Ready

Ready for testing

  
Refactor

   
Regression

   
Release

   
Repository

[Repository] Issues and PRs related to the app repo

  
Security

   
Suggestion

   
Support

   
Testing

   
Translation

   
UI/UX

   
Upstream

   
Website

[Website] Issues and PRs related to the app website

  
WIP
No Label
Priority-high
Accepting-merge-requests
API
API-dependency
Backport
Blocked
Brainstorming
Breaking
Bug
Changelog
CI
Cleanup
Confirmed
Discussion
Documentation
Duplicate
Enhancement
External-dependecy
F-droid
Feature
Google-play
Improvement
Invalid
Investigate
LGTM-done
LGTM-need
Long-term
Major-release
Minor-release
Needs-cleanup
Needs-feedback
Needs-help
Priority-critical
Priority-low
Priority-medium
Question
Ready
Refactor
Regression
Release
Repository
Security
Suggestion
Support
Testing
Translation
UI/UX
Upstream
Website
WIP
Milestone
Clear milestone
No items
No Milestone
2.4.0
Assignees
Clear assignees
No Assignees
mmarif
2 Participants
Total Time Spent: 41 minutes 24 seconds
6543
41 minutes 24 seconds
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitnex/GitNex#260
No description provided.
Delete Branch "light-theme"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?