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

Inject button/modal in new issue/PR experience structure #218

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

patrickhlauke
Copy link

Makes the extension also work in the new (currently in Beta) issue/PR experience that users can opt into at the moment.

Makes the extension also work in the new (currently in Beta) issue/PR experience that users can opt into at the moment.
@patrickhlauke
Copy link
Author

Note that I was not able to properly test this - whenever I try to just run this locally from directory, I get extension errors due to the module declarations. Would be good, if there are any particular build steps required to go from this to a working extension, if they could be documented in the README here (unless I missed them)

@patrickhlauke
Copy link
Author

patrickhlauke commented Jul 7, 2024

To test, make sure to enroll in the "Beta" experience (at the top of an /issues page)

screenshot of the 'Beta' - Try the new experience notification/button

@johnmurphy01
Copy link
Contributor

Note that I was not able to properly test this - whenever I try to just run this locally from directory, I get extension errors due to the module declarations. Would be good, if there are any particular build steps required to go from this to a working extension, if they could be documented in the README here (unless I missed them)

I'll add a Testing section to the README. But the module errors should not prevent the extension from being used. I tested last week and those errors were present. I think Chrome extensions don't like ES6 notation but I'm using that in order to run some Jest tests

@johnmurphy01
Copy link
Contributor

To test, make sure to enroll in the "Beta" experience (at the top of an /issues page)

screenshot of the 'Beta' - Try the new experience notification/button

Hmm I'm not seeing that prompt. I guess maybe it's only available to some people?

Copy link
Contributor

@johnmurphy01 johnmurphy01 left a comment

Choose a reason for hiding this comment

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

I'll wait to merge until I can verify the new DOM structure...somehow

app.js Outdated Show resolved Hide resolved
@patrickhlauke
Copy link
Author

patrickhlauke commented Jul 7, 2024

it may indeed be the case that only certain cohorts have had the beta enabled for them. have a look if you have the "Feature preview" option in the user menu, and then "Issue vNext" there

the user menu, with "Feature preview" circled

the feature preview modal, with the "Issue vNext" option shown

@patrickhlauke patrickhlauke force-pushed the patrickhlauke-new-issue-pr-experience branch 2 times, most recently from debdeb4 to d716fad Compare July 8, 2024 00:05
Original selector was stupidly incorrect. Also, as we're now inserting *after*, need to reverse the order in which we insert these, otherwise the modal will come before the button
@patrickhlauke patrickhlauke force-pushed the patrickhlauke-new-issue-pr-experience branch from d716fad to dd2de65 Compare July 8, 2024 00:11
just in case GH decide to use same/similar classname
@patrickhlauke
Copy link
Author

patrickhlauke commented Jul 8, 2024

After some tweaking (i did end up having the wrong selector), managed to get this working for the most part for local testing. The last outstanding issue I'm seeing is that the dropdown doesn't automatically close / the page doesn't reload after I choose an option in the modal...and I'm not sure where to look for the cause just yet. this may be down to the fact that the new issue experience uses React, while the previous one was Rails, so ... might not do an actual page reload/refresh of parts when things change?

kamino-new-issue-experience.mp4

@johnmurphy01
Copy link
Contributor

@patrickhlauke let me see if I can get in contact with someone at Github to enroll in the beta. I can help troubleshoot.

But can you check the network traffic after you choose a repo? It should be making github api calls, maybe one of them is failing or timing out? Although it does appear that it worked after a refresh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants