Add support for tags #1002

Merged
mmarif merged 27 commits from qwerty287/GitNex:tags into main 5 months ago
Collaborator

Closes #983

TODO

  • create tags
  • view tags
  • apply pagination
  • delete tags
Closes #983 TODO - [x] create tags - [x] view tags - [x] apply pagination - [x] delete tags
qwerty287 added 1 commit 9 months ago
qwerty287 added the
Feature
label 9 months ago
qwerty287 added this to the 4.2.0 milestone 9 months ago
Owner

You can add pagination if you would like.

You can add pagination if you would like.
Poster
Collaborator

I know, forgot in todo list

I know, forgot in todo list
qwerty287 added 5 commits 9 months ago
qwerty287 changed title from WIP: Add support for tags to Add support for tags 9 months ago
Poster
Collaborator

I think it's ready

I think it's ready
Owner

Thanks, I will review it soon when get some time.

Bdw, the pagination implementation with viewmodel will not work properly. Have you test it?

Thanks, I will review it soon when get some time. Bdw, the pagination implementation with viewmodel will not work properly. Have you test it?
Poster
Collaborator

Yes, and for me it's working (but it uses a simple workaround, just send the adapter as a parameter of the function, maybe that's what you mean)

Yes, and for me it's working (but it uses a simple workaround, just send the adapter as a parameter of the function, maybe that's what you mean)
qwerty287 added 1 commit 8 months ago
Owner

Can we add a progress indicator when we switch with filter, and for pages load?

For creating tag, If I tap on create tag. Will it create a release too or just tag?

Maybe we can enhance creating release screen to accomodate tag properly.

Can we add a progress indicator when we switch with filter, and for pages load? For creating tag, If I tap on create tag. Will it create a release too or just tag? Maybe we can enhance creating release screen to accomodate tag properly.
mmarif requested changes 8 months ago
private String repoOwner;
private String releaseTag;
private boolean viewTypeIsTags = false;
private int page = 1;
Owner

Just initlization would work, private int page;

Just initlization would work, `private int page;`
Poster
Collaborator

Well, I tried this and in this case page was 0 at the beginning, it must be 1.

Well, I tried this and in this case `page` was 0 at the beginning, it must be 1.
6543 marked this conversation as resolved
Call<List<GitTag>> call = RetrofitClient
.getApiInterface(ctx)
.getTags(token, owner, repo, 1, 50);
Owner

Page limit should come from helpers/Constants. Our default is 25 for new and 10 for old.

Page limit should come from `helpers/Constants`. Our default is 25 for new and 10 for old.
qwerty287 marked this conversation as resolved
Call<List<GitTag>> call = RetrofitClient
.getApiInterface(ctx)
.getTags(token, owner, repo, page, 50);
Owner

Page limit should come from helpers/Constants. Our default is 25 for new and 10 for old.

Page limit should come from `helpers/Constants`. Our default is 25 for new and 10 for old.
qwerty287 marked this conversation as resolved
<string name="tagCreated">Tag created</string>
<string name="asRef">Use as reference</string>
<string name="deleteTagConfirmation">Do you really want to delete this tag?</string>
<string name="deleteTagTitle">Delete tag %s</string>
Owner

How to delete a tag?

How to delete a tag?
Poster
Collaborator

There should be a button with three dots next to the tag name in the list, there is the option.

There should be a button with three dots next to the tag name in the list, there is the option.
Owner

Unfortunately I don't see one. I am on GitNex repo.

Unfortunately I don't see one. I am on GitNex repo.
Poster
Collaborator

Ok, I'll see what's wrong.

Ok, I'll see what's wrong.
Poster
Collaborator

Oh, I missed the permission system. I added the values used in #977 and currently this will always be false and it will be hidden... I'll disable the permission check so we can enable it if #977 is merged

Oh, I missed the permission system. I added the values used in #977 and currently this will always be false and it will be hidden... I'll disable the permission check so we can enable it if #977 is merged
Poster
Collaborator

Could you try again with this d27b0e0db7?

Could you try again with this https://codeberg.org/gitnex/GitNex/commit/d27b0e0db7b6c43a8ddefe686e5a5f13cb890595?
6543 marked this conversation as resolved
Owner

And while you are at it, can you add pagination to Releases too?

And while you are at it, can you add pagination to Releases too?
Poster
Collaborator

@mmarif

Can we add a progress indicator when we switch with filter, and for pages load?

Sure!

For creating tag, If I tap on create tag. Will it create a release too or just tag?

Only the tag

Maybe we can enhance creating release screen to accomodate tag properly.

I mostly copied this ui from gitea itself, but yes that would be probably good.

@mmarif >Can we add a progress indicator when we switch with filter, and for pages load? Sure! >For creating tag, If I tap on create tag. Will it create a release too or just tag? Only the tag >Maybe we can enhance creating release screen to accomodate tag properly. I mostly copied this ui from gitea itself, but yes that would be probably good.
Poster
Collaborator

@mmarif

And while you are at it, can you add pagination to Releases too?

Requires a tea4j update, but yes I can do this👍

@mmarif >And while you are at it, can you add pagination to Releases too? Requires a tea4j update, but yes I can do this👍
mmarif requested changes 8 months ago
<string name="selectUpdateStrategy">Select Update Strategy</string>
<string name="tags">Tags</string>
<string name="releasesTags">Releases/Tags</string>
<string name="create_tag">Create Tag</string>
Owner

As it is following the web UI, in that case can you change the button text to Create Tag Only

As it is following the web UI, in that case can you change the button text to `Create Tag Only`
Poster
Collaborator
https://codeberg.org/gitnex/GitNex/commit/49ce8763323c14dd75a1d822e1019e6ce12cce89
qwerty287 marked this conversation as resolved
qwerty287 added 2 commits 8 months ago
qwerty287 added 1 commit 8 months ago
qwerty287 added 1 commit 8 months ago
qwerty287 added 1 commit 8 months ago
qwerty287 added 1 commit 8 months ago
Collaborator

@qwerty287 can you add a check and only show this if gitea instance is >= 1.15 ?

@qwerty287 can you add a check and only show this if gitea instance is >= 1.15 ?
qwerty287 added 1 commit 7 months ago
qwerty287 added 2 commits 7 months ago
Collaborator

conflicts :D

conflicts :D
qwerty287 added 1 commit 6 months ago
Owner

@qwerty287 can you fix the conflicts?

@qwerty287 can you fix the conflicts?
qwerty287 added 1 commit 5 months ago
qwerty287 self-assigned this 5 months ago
mmarif added 1 commit 5 months ago
mmarif added 1 commit 5 months ago
Owner

This @string/labelMenuDelete does not exist in bottom_sheet_tag_in_list.xml. Am I missing something?

This `@string/labelMenuDelete` does not exist in `bottom_sheet_tag_in_list.xml`. Am I missing something?
Poster
Collaborator

It does in line 21 - 34?

It does in line 21 - 34?
Owner

@qwerty287

It does in line 21 - 34?

Sorry I was not clear. I mean that string is missing in strings.xml.

@qwerty287 >It does in line 21 - 34? Sorry I was not clear. I mean that string is missing in strings.xml.
qwerty287 added 1 commit 5 months ago
Poster
Collaborator

Yes, fixed it.

Yes, fixed it.
mmarif requested changes 5 months ago
enableProcessButton();
AlertDialogs.authorizationTokenRevokedDialog(ctx, ctx.getResources().getString(R.string.alertDialogTokenRevokedTitle),
ctx.getResources().getString(R.string.alertDialogTokenRevokedMessage),
ctx.getResources().getString(R.string.alertDialogTokenRevokedCopyNegativeButton),
Owner

This should be R.string.cancelButton as there is no such string for alertDialogTokenRevokedCopyNegativeButton.

This should be `R.string.cancelButton` as there is no such string for `alertDialogTokenRevokedCopyNegativeButton`.
AlertDialogs.authorizationTokenRevokedDialog(ctx, ctx.getResources().getString(R.string.alertDialogTokenRevokedTitle),
ctx.getResources().getString(R.string.alertDialogTokenRevokedMessage),
ctx.getResources().getString(R.string.alertDialogTokenRevokedCopyNegativeButton),
ctx.getResources().getString(R.string.alertDialogTokenRevokedCopyPositiveButton));
Owner

This should be R.string.navLogout instead of alertDialogTokenRevokedCopyPositiveButton. No such string.

This should be `R.string.navLogout` instead of `alertDialogTokenRevokedCopyPositiveButton`. No such string.
Owner

Can you help build the app from this PR to fix the missing bits. Thanks

Can you help build the app from this PR to fix the missing bits. Thanks
mmarif added 1 commit 5 months ago
qwerty287 added 2 commits 5 months ago
5661fd2423
Hide actions if the user can't use them (#977)
qwerty287 added 2 commits 5 months ago
mmarif added 1 commit 5 months ago
Owner

@qwerty287 this is ready for mergning. Just one thing I wish was there, when delete a tag, refresh the view with updated list. But it can be done in another PR.

@qwerty287 this is ready for mergning. Just one thing I wish was there, when delete a tag, refresh the view with updated list. But it can be done in another PR.
mmarif approved these changes 5 months ago
mmarif merged commit f79d227bff into main 5 months ago
mmarif referenced this issue from a commit 5 months ago
qwerty287 deleted branch tags 5 months ago

Reviewers

mmarif approved these changes 5 months ago
The pull request has been merged as f79d227bff.
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.