DEVELOP-692: Add copy link for branch on github extension#14
Conversation
… to switch to that branch
cornu-ammonis
left a comment
cornu-ammonis
left a comment
There was a problem hiding this comment.
nice refactors! a few questions/comments
| import { useGithubApi } from "../lib/useGithubApi"; | ||
| import { usePopperAlerter } from "../lib/usePopperAlerter"; | ||
| import ExternalLink from "./ExternalLink"; | ||
| import { ExternalLink } from "./ExternalLink"; |
There was a problem hiding this comment.
curious why to change this from default export? just a style preference?
There was a problem hiding this comment.
We had some one way, some the other. I just normalized them all to be one way and chose the majority as consensus. Named exports have an advantage imo in that you must refer to their name in the import, making them more greppable. I've run across a couple in aha-app where the component/file had one name and the import referenced it with another.
There was a problem hiding this comment.
makes sense, it does bug me when the import differs from the filename for a default export.
| <aha-icon icon="fa-regular fa-code-branch type-icon" /> | ||
| </span> | ||
| <ExternalLink href={branch.url}>{branch.name}</ExternalLink> | ||
| <a |
There was a problem hiding this comment.
I think this would be more accessible as a button rather than <a, but maybe that would introduce some other difficulties?
There was a problem hiding this comment.
yeah, it looks a bit crappy as a button. I did try that out. As a link it looks pretty good
2 participants
Add a copy link to github branches in the attribute to make it easier to switch to that branch https://github.com/aha-develop/github/pull/14/files#diff-a8a1f4ed71f557f835fbb57d479877141b86cbb25a9e330abf7cdfd1b383bcc0R8
To make it work better I've also updated the linking so that when a PR is linked it's branch is also linked https://github.com/aha-develop/github/pull/14/files?diff=split&w=1#diff-28e5f92f2fbcf6ab1be7afe60b1c7fe70c742c37b9e4fc1def7762fd4b32d0a7R99 as often the branch name does not match the Aha! reference when the PR does.
copy-2021-05-24_17.36.27.webm.mp4
I've also refactored the code a little by pulling the attribute and page components in their own folders. While I was doing that I made the way components are exported the same (named) so there wasn't a confusing mix of default and named exports.