Add option to update PR #973

Merged
mmarif merged 11 commits from qwerty287/GitNex:update-pr into main 2 weeks ago

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

Closes #967


### Describe what your pull request does and which issue you’re targeting <!-- Create a new issue, if it doesn't exist yet --> Closes #967 <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 3 commits 4 weeks ago
mmarif added the
Enhancement
label 4 weeks ago
mmarif added this to the 4.1.0 milestone 4 weeks ago
Collaborator

INFO: on gitea v1.16 and onwoards we will have a rebase update option

(https://github.com/go-gitea/gitea/pull/16125)

INFO: on gitea v1.16 and onwoards we will have a rebase update option (https://github.com/go-gitea/gitea/pull/16125)
Poster

@6543 is there an API to do this?
My local Gitea instance is actually build on top of Gitea's main branch and I didn't see an option in the swagger

@6543 is there an API to do this? My local Gitea instance is actually build on top of Gitea's `main` branch and I didn't see an option in the swagger
Poster

OK, found it. I could implement this too, any UI ideas? A second option in the bottom sheet wouldn't be that nice, maybe an AlertDialog or a seperate bottom sheet?

OK, found it. I could implement this too, any UI ideas? A second option in the bottom sheet wouldn't be that nice, maybe an AlertDialog or a seperate bottom sheet?
qwerty287 changed title from Add option to update PR to Wip: Add option to update PR 4 weeks ago
Owner

Current BottomSheet entry could be renamed to Update Pull Request, on tap it will trigger an alert diaglog with two options. This way seems more logical and cleaner.

Regarding the feature, as it will be available in 1.16 if I am not mistaken, a version check will be needed.

Current BottomSheet entry could be renamed to `Update Pull Request`, on tap it will trigger an alert diaglog with two options. This way seems more logical and cleaner. Regarding the feature, as it will be available in 1.16 if I am not mistaken, a version check will be needed.
Poster

@mmarif This is working with an URL argument, which will be ignored if it's not supported by Gitea, right? (But yes, for the ui we'll need a version check)

@mmarif This is working with an URL argument, which will be ignored if it's not supported by Gitea, right? (But yes, for the ui we'll need a version check)
Owner

@qwerty287 I believe query is fine and will be ignored, but as mentioned it needs to be handled on the app side(UI).

@qwerty287 I believe query is fine and will be ignored, but as mentioned it needs to be handled on the app side(UI).
qwerty287 added 1 commit 3 weeks ago
qwerty287 added 1 commit 3 weeks ago
qwerty287 changed title from Wip: Add option to update PR to Add option to update PR 3 weeks ago
mmarif requested changes 3 weeks ago
new AlertDialog.Builder(context)
.setTitle(R.string.selectUpdateStrategy)
.setNeutralButton(R.string.cancelButton, null)
.setItems(new String[]{context.getString(R.string.updateStrategyMerge), context.getString(R.string.updateStrategyRebase)},
mmarif commented 3 weeks ago
Poster
Owner

I don't know if it would be more cleaner and nice if we can have two buttons instead OR something similar because currently it looks very rough. Any other ideas?

I don't know if it would be more cleaner and nice if we can have two buttons instead OR something similar because currently it looks very rough. Any other ideas?
Poster

The only idea I'd have is to use two buttons with an image - you could look at https://qwerty287.codeberg.page/button_example.html for an example in HTML, we could do this just with a better design and rebase and merge texts and fitting icons.

The only idea I'd have is to use two buttons with an image - you could look at https://qwerty287.codeberg.page/button_example.html for an example in HTML, we could do this just with a better design and `rebase` and `merge` texts and fitting icons.
qwerty287 marked this conversation as resolved
<string name="unfollowingFailed">Couldn\'t unfollow user</string>
<string name="followingFailed">Couldn\'t follow user</string>
<string name="updatePrConflict">The pull request conflicts with the base branch. Please resolve the conflicts and try again.</string>
<string name="updatePrSuccess">Pull request updated successfully</string>
mmarif commented 3 weeks ago
Poster
Owner

Change to Pull Request updated successfully

Change to `Pull Request updated successfully`
qwerty287 marked this conversation as resolved
<string name="updatePrSuccess">Pull request updated successfully</string>
<string name="updateStrategyMerge">Merge</string>
<string name="updateStrategyRebase">Rebase</string>
<string name="selectUpdateStrategy">Select update strategy</string>
mmarif commented 3 weeks ago
Poster
Owner

Change to Select Update Strategy

Change to `Select Update Strategy`
qwerty287 marked this conversation as resolved
qwerty287 added 1 commit 3 weeks ago
Owner

Something like this?

I know it's not a very good UX/UI but still better than plain text. :)

Something like this? I know it's not a very good UX/UI but still better than plain text. :)
17 KiB
Poster

My first draft is this one, would be nice if you could give feedback

My first draft is this one, would be nice if you could give feedback
Owner

This is great, just the lines are missing though. Most of the new popups we use has header and footer lines as a separator.

This is great, just the lines are missing though. Most of the new popups we use has header and footer lines as a separator.
qwerty287 added 2 commits 3 weeks ago
qwerty287 added 1 commit 3 weeks ago
qwerty287 added 1 commit 2 weeks ago
mmarif added 1 commit 2 weeks ago
mmarif approved these changes 2 weeks ago
mmarif merged commit 95aea16a07 into main 2 weeks ago
mmarif referenced this issue from a commit 2 weeks ago
qwerty287 deleted branch update-pr 2 weeks ago

Reviewers

mmarif approved these changes 2 weeks ago
The pull request has been merged as 95aea16a07.
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.