feat: Suggest integration after pasting link #7664
Conversation
gitstart-parabol
changed the title
There was a problem hiding this comment.
Thank you for the PR. It seems to handle only one scenario ie. Github, where the issue says When a user pastes a link from one of the integrations that we support.... It'd be great to have a more generic solution, that'd detect any of the integrations Parabol supports.
Do you mind taking a look and outlining the proposed solution in the issue itself, before writing code? It'll make us on the same page and hopefully resolve more unknowns and save some time during the review. Thank you!
|
@BartoszJarocki we have developed a broader solution to cover all integrations, please review. |
| const [integration, setIntegration] = React.useState<Integration | undefined>() | ||
|
|
||
| React.useEffect(() => { | ||
| const text = editorState.getCurrentContent().getPlainText() |
There was a problem hiding this comment.
-1 it's better to extract a variable above like
const plainTextValue = editorState.getCurrentContent().getPlainText()
and use it in the hook and hook deps array to avoid calling it twice, especially considering it's pretty heavyweight method
|
|
||
| useEffect(() => { | ||
| const getProviders = async () => { | ||
| const result = await atmosphere.fetchQuery<useIntegrationProvidersGetProvidersListQuery>( |
There was a problem hiding this comment.
-1 fetching data in useEffect isn't something we should be doing when using relay. this hook should have it's own fragment, and the fragment should be spread in each component that's using this hook
There was a problem hiding this comment.
Hi @BartoszJarocki, wasn't able to spread the fragment specifically for teamMember.integrations in OutcomeCard.tsx because doing so would require making extensive changes throughout other fragments in the same hierarchy just to include the teamId argument. As a result, we have instead created a new root for the providers list query in the IntegrationsBanner
| query useIntegrationProvidersGetProvidersListQuery($teamId: ID!) { | ||
| viewer { | ||
| teamMember(teamId: $teamId) { | ||
| integrations { |
There was a problem hiding this comment.
-1 preferably, this data should be the same as in ProviderList so I'd encurage to reuse that fragment
| name: string | ||
| connected: boolean | ||
| regex: RegExp | ||
| provider?: any |
| const SuggestIntegration = styled('div')({ | ||
| display: 'flex', | ||
| flexDirection: 'row', | ||
| border: `1px solid ${PALETTE.SLATE_400}`, | ||
| borderRadius: '4px', | ||
| marginTop: '8px', | ||
| padding: '8px 6px' | ||
| }) | ||
|
|
||
| const Link = styled('span')({ | ||
| color: PALETTE.SLATE_800, | ||
| cursor: 'pointer', | ||
| textDecoration: 'underline', | ||
| fontWeight: 600, | ||
| ':hover, :focus, :active': { | ||
| color: PALETTE.SLATE_600 | ||
| } | ||
| }) | ||
|
|
||
| const SuggestionContent = styled('div')({ | ||
| padding: 0 | ||
| }) | ||
|
|
||
| const CloseIcon = styled(Close)({ | ||
| color: PALETTE.SLATE_600, | ||
| cursor: 'pointer' | ||
| }) |
There was a problem hiding this comment.
+1 we're migrating to TailwindCSS, so if possible all new stuff should be done using Tailwind
|
Hi! Thank you for contributing to our app! We're closing this PR due to inactivity, but that doesn't mean the end of the code! This is still a useful feature & if we prioritize it in an upcoming sprint we will use this PR as a reference point. |
Description
Fixes #5940
Suggest integration after pasting link
When a user pastes a link from:
Demo
Trimed.mov
Testing scenarios
Final checklist
One Review Requiredif the PR introduces only minor changes, does not contain any architectural changes or does not introduce any new patterns and I think one review is sufficient'