Merged
mmarif
merged 14 commits from bottomsheets-actions-search-filters-apilimit-tags into main 2 months ago
6543
qwerty287
mmarif
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'bottomsheets-actions-search-filters-apilimit-tags'
Deleting a branch is permanent. It CANNOT be undone. Continue?
PR targeting multiple issues: #1101, #1092, #1080, #1079, #1057to PR targeting multiple issues: #1101, #1092, #1080, #1079, #1057 2 months agoPR targeting multiple issues: #1101, #1092, #1080, #1079, #1057to [WIP] PR targeting multiple issues: #1101, #1092, #1080, #1079, #1057 2 months ago[WIP] PR targeting multiple issues: #1101, #1092, #1080, #1079, #1057to PR targeting multiple issues: #1101, #1092, #1080, #1079, #1057 2 months agoThis PR is ready for testing.
got my lgtm
@mmarif please resolve conflicts :)
@6543 done. :)
private String spinnerSelectedValue;private Version giteaVersion;private int maxResponseItems = 50;private int defaultPagingNumber = 25;Gitea's default for this is 30.
@Overridepublic void migrate(@NonNull SupportSQLiteDatabase database) {database.execSQL("ALTER TABLE 'userAccounts' ADD COLUMN 'maxResponseItems' INTEGER NOT NULL DEFAULT 50");database.execSQL("ALTER TABLE 'userAccounts' ADD COLUMN 'defaultPagingNumber' INTEGER NOT NULL DEFAULT 25");Gitea's default for this is 30, we should adapt this.
public static int getCurrentResultLimit(Context context) {return ((BaseActivity) context).getAccount().requiresVersion("1.12") ? resultLimitNewGiteaInstances : resultLimitOldGiteaInstances;return ((BaseActivity) context).getAccount().requiresVersion("1.15") ? ((BaseActivity) context).getAccount().getDefaultPageLimit() : ((BaseActivity) context).getAccount().getMaxPageLimit();Maybe I missed something at Gitea here, but why can't we always use max limit? Is the max limit filed new in 1.15?
And BTW, support for Gitea < 1.14 is dropped, so this condition can be safely removed.
Let's keep the check for now. We can remove it in later releases.
android:gravity="center"app:layout_alignSelf="flex_start"app:drawableTopCompat="@drawable/ic_browser"android:text="@string/isOpen"That's too short. "Open" as text can say anything. At least, the browser must appear here in the string (also in all the other usages).
Even though the icon is self explanatory, but for the context I have added browser to it.
android:visibility="gone"android:textSize="16sp" /><TextViewNot related, but can you hide this view if the repo is archived? There is a condition in the BottomSheet fragment, just add it here.
It already have this
android:visibility="gone". Am I missing something? Or you mean a check in the bottomsheet?Added a check.
android:textColor="?attr/primaryTextColor"android:textSize="16sp" /><TextViewNot related, but can you hide this view if the repo is archived? There is a condition in the BottomSheet fragment, just add it here.
android:textColor="?attr/primaryTextColor"android:textSize="16sp" /><TextViewNot related, but can you hide this view if the repo is archived? There is a condition in the BottomSheet fragment, just add it here.
This actually needs a check if the head repo is archived, the head repo is already requested in the issue detail activity, just store and check the value here.
android:gravity="center"app:layout_alignSelf="flex_start"app:drawableTopCompat="@drawable/ic_branch"android:text="@string/menuDeleteText"I'm not sure if this is a good idea. Only "delete" is not enough - the icon doesn't say everything. In case of a repository, that's fine because most will now the repo icon, but the branch icon isn't that clear. I understand your thoughts here, but this is not really good in terms of accessibility.
android:textColor="?attr/primaryTextColor"android:textSize="16sp" /></com.google.android.flexbox.FlexboxLayout>What about share/open in browser/copy here?
As this part is more of a diverse, adding a specific copy will not suffice. In repo it's fine to say Repository as we have repo specific things like Settings. But here I am not sure. If we want to add a header copy, it should be more generic to cover all the used items.
<?xml version="1.0" encoding="utf-8"?><LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"This if fine for me now, but later I would like to change it. I like the Octodroid way, I'll try to add a screenshot (can't add it on the reviews).
#1109
Screenshot of Octodroids PR "dashboard". I only refer to the branch views, not the general tabbed layout etc.
@qwerty287 Most of your ideas are implemented.
As commented by me, if you have more generic copy(text) we can add that. I am out of luck for it. :)
When I was implementing it I thought of inline usage like other items we currently have, but then I thought if what we have more info to add in the future. So instead of cluterring the main view, I preferred to have info icon where we add stuff and expand it later.
Octodrioid implementation is fine too. If you think we should be adding stuff(more) in the main view. I can change it or you can change it in this PR.
If the review changes are working (didn't test), LGTM.
Let's keep it for now I'll review it in 4.3.0 - I never worked with a dialog-based system, so I'll try first. If it doesn't work well for anybody, we can change it later.
c67b3c178finto main 2 months agoReviewers
c67b3c178f.