Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use native style in update-pr-from-base-branch #6537

Merged
merged 13 commits into from Apr 26, 2023
Merged

Use native style in update-pr-from-base-branch #6537

merged 13 commits into from Apr 26, 2023

Conversation

fregante
Copy link
Member

@fregante fregante commented Apr 17, 2023

TODO:

  • Readme update

PR without conflicts

refined-github/sandbox#60

Screenshot 14

Draft PR without conflicts

refined-github/sandbox#61

Screenshot 16

Native "Update branch" button

#6531

(or pick a conflict-free PR from https://github.com/refined-github/refined-github/pulls?q=is%3Apr+is%3Aopen+sort%3Acreated-asc)

Screenshot

Native "Resolve conflicts" button

refined-github/sandbox#9

Screenshot

features.unload(import.meta.url);

async function handler(event: DelegateEvent<MouseEvent, HTMLButtonElement>): Promise<void> {
event.delegateTarget.disabled = true;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet tested

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works but we actually need to remove it after, or else:

fixed

I added a .remove()


const selectorForPushablePRNotice = '.merge-pr > .color-fg-muted:first-child';
const canMerge = '.merge-pr > .color-fg-muted:first-child';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing it in our own repo though 🤷‍♂️

mergeabilityRow.prepend(

<div
className="branch-action-btn float-right js-immediate-updates js-needs-timeline-marker-header"
Copy link
Member Author

@fregante fregante Apr 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if these last 2 classes are useful, I just copied the native row and dropped every class and element I understood.

icon: <CheckIcon/>,
iconClass: 'completeness-indicator-success',
heading: 'This branch has no conflicts with the base branch',
meta: 'Merging can be performed automatically.',
Copy link
Member Author

@fregante fregante Apr 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this meta line because without it it's a little misaligned, but I'm not totally sure it's a always true statement exactly as GitHub defines it.

Eventually this line will likely contain:

@fregante fregante changed the title Use native style for update-pr-from-base-branch Use native style in update-pr-from-base-branch Apr 17, 2023
@fregante fregante mentioned this pull request Apr 17, 2023
1 task
@@ -279,7 +279,7 @@ Thanks for contributing! 🦋🙌
### Editing pull requests

- [](# "sync-pr-commit-title") 🔥 [Uses the PR’s title as the default squash commit title](https://github.com/refined-github/refined-github/issues/276) and [updates the PR’s title to match the commit title, if changed.](https://user-images.githubusercontent.com/1402241/51669708-9a712400-1ff7-11e9-913a-ac1ea1050975.png)
- [](# "update-pr-from-base-branch") [Adds a button to update a PR from the base branch to ensure it builds correctly before merging the PR itself.](https://user-images.githubusercontent.com/1402241/106494023-816d9a00-647f-11eb-8cb1-7c97aa8a5546.png) GitHub only adds it when the base branch is "protected".
- [](# "update-pr-from-base-branch") [Adds an "Update branch" button to every PR.](https://user-images.githubusercontent.com/1402241/234483592-4867cb2e-21cb-436d-9ea0-aedadf834f19.png) GitHub has the same feature, but it must be manually configured with protected branches.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tooltip is actually slightly pushed to the left, not aligned right. I only did it manually here to make it look better.

It can't be centered because it's cut off on sidebar-less views. I tried tooltipped-e but it looked bad too

Screenshot 12

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tooltip was added to clarify that this is a RGH feature, because it blends in way too well otherwise.

@fregante fregante merged commit 7e94056 into main Apr 26, 2023
10 checks passed
@fregante fregante deleted the update-pr branch April 26, 2023 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Use native style for update-pr-from-base-branch
1 participant