Enable add-ons for Omnisend discoverability #66
Conversation
|
|
||
| public static function omnisend_post_connection() { | ||
| $connected = Options::is_store_connected(); | ||
| // phpcs:ignore WordPress.WP.CapitalPDangit.MisspelledInText |
There was a problem hiding this comment.
maybe add some comment why this ignore is used
| 3. Paste created API key here: | ||
| </div> | ||
| </div> | ||
| <Flex align={"'start'"} gap={4} wrap="true"> |
There was a problem hiding this comment.
| <Flex align={"'start'"} gap={4} wrap="true"> | |
| <Flex align="align" gap={4} wrap="true"> |
|
|
||
| return ( | ||
| <> | ||
| <div className="omnisend-spacing-mv-8"> |
There was a problem hiding this comment.
why are we not using lib spacer?
There was a problem hiding this comment.
because it is under experimental components:
https://wordpress.github.io/gutenberg/?path=/docs/components-experimental-spacer--docs
| import ConnectionLogos from './connection-logos'; | ||
| import ConnectionFeatures from './connection-features'; | ||
| import ConnectionSteps from './connection-steps'; | ||
| import { useState } from '@wordpress/element'; |
There was a problem hiding this comment.
minor: we probably can import react directly instead using this wrapper WordPress/gutenberg#54074 (comment)
There was a problem hiding this comment.
'react' should be listed in the project's dependencies.
In our case, it is only peerDependencies
| const [error, setError] = useState(null); | ||
| const [loading, setLoading] = useState(null); | ||
|
|
||
| const connect = (apiKey) => { |
There was a problem hiding this comment.
connect => handleSiteConnection
| .then((response) => response.json()) | ||
| .then((data) => { | ||
| if (data.success) { | ||
| location.reload(); |
There was a problem hiding this comment.
would prefer to start using react router and simply redirect instead of have both react & php routing
| } | ||
| }) | ||
| .catch((e) => { | ||
| setError(e.message || e); |
There was a problem hiding this comment.
The error sholuld be clear for the user, wont this result in some unclear timeout error?
omnisend/package.json
Outdated
| { | ||
| "name": "wp-omnisend", | ||
| "version": "1.0.0", | ||
| "description": "A pre-publish checklist for content that includes a customizable settings page.", |
Co-authored-by: Arnas Gečas <13507001+arnas@users.noreply.github.com>
nerijuszaniauskas
changed the title
|
|
||
| add_action( 'admin_notices', 'Omnisend_Core_Bootstrap::admin_notices' ); | ||
| add_action( 'admin_menu', 'Omnisend_Core_Bootstrap::add_admin_menu' ); | ||
| add_action( 'admin_enqueue_scripts', 'Omnisend_Core_Bootstrap::load_omnisend_admin_styles' ); | ||
|
|
||
| add_action( 'admin_init', 'Omnisend\Internal\Connection::connect_with_omnisend_for_woo_plugin' ); | ||
|
|
||
| add_action( | ||
| 'admin_enqueue_scripts', |
There was a problem hiding this comment.
Maybe this three can be extracted to function?
| @@ -72,6 +133,21 @@ function ( $user_login, $user ) { | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| public static function omnisend_app_market() { | |||
There was a problem hiding this comment.
Spacing not correct.
| ); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error('Failed to load apps'); |
There was a problem hiding this comment.
Can er display apps from local file instead of error?
https://omnisend.atlassian.net/browse/INT-754