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

Fallback to AMP Legacy Reader theme if active Reader theme is unavailable #5159

Merged
merged 38 commits into from
Aug 8, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
fdedae8
Show mobile icon placeholder if none was defined
pierlon Jul 31, 2020
3c9b293
Show desktop icon placeholder if none was defined
pierlon Aug 3, 2020
fd55eb7
Obtain active reader theme data from list of installed themes if avai…
pierlon Aug 4, 2020
8b6b1e9
Fallback to legacy Reader mode if active Reader theme is unavailable
pierlon Aug 4, 2020
289dce4
Fix lint errors
pierlon Aug 4, 2020
3d39560
Revert making Reader theme service conditional
pierlon Aug 6, 2020
aeeb22e
Fix how legacy mode is determined
pierlon Aug 6, 2020
030df93
Merge branch 'develop' into fix/5070-custom-reader-theme-info
pierlon Aug 7, 2020
688b8b2
Center desktop screenshots when viewed in mobile viewport
pierlon Aug 7, 2020
b76fe2d
Align SVG content to canvas
pierlon Aug 7, 2020
a9cba2b
Show an admin notice on Settings page if Reader theme becomes unavail…
pierlon Aug 7, 2020
bf6ca59
Add test to verify Reader theme data can be retrieved from list of in…
pierlon Aug 7, 2020
3575517
Show mobile image placeholder in onboarding wizard
pierlon Aug 7, 2020
5a71cf6
Use already existing reader theme
pierlon Aug 7, 2020
6c34896
Fix test-amp-helper-functions for core themes in src
westonruter Aug 7, 2020
05a673d
Simplify logic in amp_is_legacy()
westonruter Aug 7, 2020
d03eaf9
Let child-of-core be child of Twenty Seventeen to fix WP<5.3 tests
westonruter Aug 7, 2020
e8e1d93
Fix tests in WP 4.9 and ensure core themes in src are registered
westonruter Aug 7, 2020
b2f4ed0
Programmatically select the AMP Legacy theme if it's used as a fallback
pierlon Aug 7, 2020
f3707a1
Make registration of core theme directory in tests DRY
pierlon Aug 7, 2020
1d939c4
Add link to reader themes drawer
westonruter Aug 7, 2020
17261a3
Restrict showing legacy fallback notice to admins on themes screen an…
westonruter Aug 7, 2020
6bb44d7
Only programmmatically select legacy theme when current Reader theme …
pierlon Aug 7, 2020
e5e086f
Ensure AMP legacy theme is being used as a fallback
pierlon Aug 7, 2020
c947bc9
Remove user capability check from ReaderThemes::using_fallback_theme
pierlon Aug 7, 2020
b2a9716
Replace astra with neve for testing since astra is suspended
westonruter Aug 7, 2020
0335d92
Enable the "Customize Theme" button after installing the new Reader t…
pierlon Aug 7, 2020
b322c37
Remove screenshot from being required for DesktopScreenshot.propTypes
westonruter Aug 7, 2020
3f005bf
Remove outdated hook dependencies
pierlon Aug 7, 2020
88fa245
Add tests for admin notice printers
westonruter Aug 8, 2020
5f65e7d
Add test for ReaderThemes::using_fallback_theme
westonruter Aug 8, 2020
7029023
Remove unused asset dependency
pierlon Aug 8, 2020
cda8156
Explicitly ignore legacy theme since not yet pushed to array
westonruter Aug 8, 2020
06ac5ed
Make ReaderThemes::normalize_theme_data() private
westonruter Aug 8, 2020
83cb2c5
Reuse variable for condition
westonruter Aug 8, 2020
877b4ab
Use LoadsCoreThemes in remaining tests
westonruter Aug 8, 2020
f2df5c7
Delete obsolete deletion of theme_roots site transient
westonruter Aug 8, 2020
810ec5f
Mock Reader themes provider
pierlon Aug 8, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* External dependencies
*/
import PropTypes from 'prop-types';

/**
* WordPress dependencies
*/
import { createContext } from '@wordpress/element';

export const ReaderThemes = createContext();

/**
* MOCK.
*
* @param {Object} props
* @param {any} props.children Component children.
* @param {boolean} props.downloadingTheme Whether downloading a theme or not.
*/
export function ReaderThemesContextProvider( { children, downloadingTheme = false } ) {
return (
<ReaderThemes.Provider value={
{
downloadingTheme,
}
}>
{ children }
</ReaderThemes.Provider>
);
}
ReaderThemesContextProvider.propTypes = {
children: PropTypes.any,
downloadingTheme: PropTypes.bool,
};
9 changes: 7 additions & 2 deletions assets/src/onboarding-wizard/components/nav/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import { Nav } from '..';
import { NavigationContextProvider } from '../../navigation-context-provider';
import { UserContextProvider } from '../../user-context-provider';
import { OptionsContextProvider } from '../../../../components/options-context-provider';
import { ReaderThemesContextProvider } from '../../../../components/reader-themes-context-provider';
import { STANDARD, READER } from '../../../../common/constants';

jest.mock( '../../../../components/options-context-provider' );
jest.mock( '../../../../components/reader-themes-context-provider' );
jest.mock( '../../user-context-provider' );

let container;
Expand All @@ -35,11 +37,13 @@ const testPages = [
{ PageComponent: MyPageComponent, slug: 'slug-2', title: 'Page 1' },
];

const Providers = ( { children, pages, themeSupport = READER } ) => (
const Providers = ( { children, pages, themeSupport = READER, downloadingTheme = false } ) => (
<OptionsContextProvider themeSupport={ themeSupport }>
<UserContextProvider>
<NavigationContextProvider pages={ pages }>
{ children }
<ReaderThemesContextProvider downloadingTheme={ downloadingTheme }>
{ children }
</ReaderThemesContextProvider>
</NavigationContextProvider>
</UserContextProvider>
</OptionsContextProvider>
Expand All @@ -48,6 +52,7 @@ Providers.propTypes = {
children: PropTypes.any,
pages: PropTypes.array,
themeSupport: PropTypes.string,
downloadingTheme: PropTypes.bool,
};

describe( 'Nav', () => {
Expand Down
3 changes: 3 additions & 0 deletions tests/js/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ module.exports = {
'<rootDir>/build/',
'<rootDir>/tests/shared',
],
modulePathIgnorePatterns: [
'<rootDir>/assets/src/components/.*/__mocks__',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I received the following warning while running the JS tests:

jest-haste-map: duplicate manual mock found: index
  The following files share their name; please delete one of them:
    * <rootDir>/assets/src/components/options-context-provider/__mocks__/index.js
    * <rootDir>/assets/src/components/reader-themes-context-provider/__mocks__/index.js

It seems Jest doesn't allow mocks with the same base name for some weird reason. It's been filed as a bug for quite some time now.

Ignoring the mocks in assets/src/components seems to resolve the issue, but I'd like to get a second opinion from @johnwatkins0 on this.

],
coverageReporters: [ 'lcov' ],
coverageDirectory: '<rootDir>/build/logs',
reporters: [ [ 'jest-silent-reporter', { useDots: true } ] ],
Expand Down