Skip to content

feat(app-platform): Integration "Learn More" modal #12638

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

Merged
merged 1 commit into from
Apr 15, 2019

Conversation

mnoble
Copy link
Contributor

@mnoble mnoble commented Apr 3, 2019

Adds a modal for when the User clicks "Learn More" on the Integrations page, for a Sentry App.

image

NOTE: The "By undefined" will say "By <author>" once #12622 is merged and out.

@mnoble mnoble requested a review from MeredithAnya April 3, 2019 21:10
@mnoble mnoble force-pushed the app-platform/learn-more branch 4 times, most recently from f43861a to 0bb7d0a Compare April 4, 2019 02:28
@mnoble mnoble force-pushed the app-platform/learn-more branch 2 times, most recently from 03de79f to 78151d3 Compare April 4, 2019 19:57

return (
<React.Fragment>
<Flex align="center" mb={2}>
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming this is copy-pasta from the other modals, but it seems weird that we have some inline styling and then some using the styled components. I feel like they should all be styled components

@@ -92,7 +92,7 @@ describe('Organization Developer Settings', function() {
body: [sentryApp],
});
const wrapper = mount(
<OrganizationDeveloperSettings params={{orgId: newOrg.slug}} />,
<OrganizationDeveloperSettings params={{orgId: newOrg.slug}} organization={org} />,
Copy link
Member

Choose a reason for hiding this comment

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

should organization={org} be using the newOrg?

@MeredithAnya
Copy link
Member

Can we add a test for the actual modal itself? Mainly to check if the button is disabled if the integration is already installed.

@mnoble mnoble force-pushed the app-platform/learn-more branch from 78151d3 to 67169a1 Compare April 9, 2019 22:59
@mnoble mnoble force-pushed the app-platform/learn-more branch 5 times, most recently from 7b57983 to ce39698 Compare April 15, 2019 18:05
Adds a modal for when the User clicks "Learn More" on the Integrations
page, for a Sentry App.
@mnoble mnoble force-pushed the app-platform/learn-more branch from ce39698 to 344a5cb Compare April 15, 2019 19:45
@mnoble mnoble merged commit b5e6ef7 into master Apr 15, 2019
@mnoble mnoble deleted the app-platform/learn-more branch April 15, 2019 20:15
jan-auer added a commit that referenced this pull request Apr 16, 2019
* master: (50 commits)
  fix(ui) Don't show save-org-search on event search (#12785)
  ref(ui): Remove some unnecessary index.jsx files (#12606)
  feat(app-platform): Analytics (#12718)
  ref(js): Remove ApiMixin (#12384)
  test(js): Silence project plugin console info spam (#12761)
  test(js): Move SaveSearchStore.reset() (#12769)
  test(js): Add more fields to Group fixture (#12759)
  feat(app-platform): Integration "Learn More" modal (#12638)
  feat(saved-searches) Move create saved search button to search bar. (#12781)
  ref(global-header): Remove dead code (#12767)
  ref(releases): Refactored Releases Serializers (#12535)
  feat(app-platform): Sort Integrations (#12697)
  ref(audit-log): Log sso config updates (#12744)
  ref(app-platform): New 'Open In' UI  (#12621)
  feat(events): Use SnubaEvent if option is turned on (#12594)
  feat(global-selection-header): show settings icon link in single project mode (#12772)
  refs(api): Consolidate all search backend code into `SnubaSearchBackend`
  fix(tests) Remove large snapshots (#12766)
  fix: Update symbolicator snapshots (#12710)
  ref: Upgrade semaphore (#12751)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants