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

Data Structure: reduce redundancy when storing fonts #12625

Merged
merged 52 commits into from
Dec 2, 2022
Merged
Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
a576668
update font storage
timarney Nov 4, 2022
11fbcfe
updateStoryFonts
timarney Nov 5, 2022
64d6a85
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 5, 2022
32c114a
clean font props when saving
timarney Nov 5, 2022
17880fa
update clean fonts
timarney Nov 5, 2022
40c9ac5
update populate
timarney Nov 5, 2022
ba32d9a
use let
timarney Nov 7, 2022
2e51f43
add test
timarney Nov 7, 2022
cb78552
add test
timarney Nov 7, 2022
494efd8
update cc
timarney Nov 7, 2022
e312df2
test debug
timarney Nov 7, 2022
535d371
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 7, 2022
4fd511d
test debug
timarney Nov 7, 2022
a36b7d0
re-add update story
timarney Nov 7, 2022
1b62f1d
temp remove migration
timarney Nov 9, 2022
a02a5e9
temp remove migration
timarney Nov 9, 2022
1e7e831
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 9, 2022
a12541b
update test
timarney Nov 9, 2022
1dcf654
re-add clean fonts
timarney Nov 9, 2022
6332a83
add font data
timarney Nov 9, 2022
efada4f
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 10, 2022
d1fae57
update migration
timarney Nov 10, 2022
a1c055a
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 11, 2022
3f4d563
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 14, 2022
6eb87c9
remove prop
timarney Nov 14, 2022
5601879
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 19, 2022
45bd4a3
update story fonts when saving props
timarney Nov 19, 2022
6194138
cleanup
timarney Nov 19, 2022
32ecb01
lint
timarney Nov 19, 2022
a1682aa
update test
timarney Nov 19, 2022
9ede80c
test updates
timarney Nov 19, 2022
46d77db
check for fonts prop
timarney Nov 19, 2022
de60cf7
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 21, 2022
9621d20
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 22, 2022
5b471db
add populateElementFontData to output
timarney Nov 22, 2022
2f0b080
check fonts length
timarney Nov 22, 2022
5c7a403
check fonts length
timarney Nov 22, 2022
0537bca
remove duplicate code
timarney Nov 22, 2022
ef95d25
remove reducer
timarney Nov 23, 2022
99f7cdd
remove mock
timarney Nov 23, 2022
34a4837
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 23, 2022
baf785d
update migration + add test
timarney Nov 23, 2022
75ca3d9
move types
timarney Nov 23, 2022
60bbe9d
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 23, 2022
4a324f9
remove migration
timarney Nov 25, 2022
1d51661
add comments
timarney Nov 25, 2022
1c06c3d
add comments
timarney Nov 25, 2022
b5c8d20
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 28, 2022
ddc8a9c
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 28, 2022
50dce47
Merge branch 'main' into re-op-element-font-data-1
timarney Nov 29, 2022
2c407a3
merge
timarney Nov 29, 2022
373f526
Merge branch 'main' into re-op-element-font-data-1
timarney Dec 1, 2022
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
1 change: 1 addition & 0 deletions packages/output/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@

export { default as OutputStory } from './story';
export { default as getStoryMarkup } from './utils/getStoryMarkup';
export { populateElementFontData } from './utils/populateElementFontData';
export { default as getTextElementTagNames } from './utils/getTextElementTagNames';
export * from './constants';
8 changes: 8 additions & 0 deletions packages/output/src/story.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@ import CustomCSS from './utils/styles';
import FontDeclarations from './utils/fontDeclarations';
import OutputPage from './page';
import getPreloadResources from './utils/getPreloadResources';
import { populateElementFontData } from './utils/populateElementFontData';

function OutputStory({
story: {
featuredMedia,
link,
title,
fonts,
autoAdvance,
defaultPageDuration,
backgroundAudio,
Expand All @@ -51,6 +53,11 @@ function OutputStory({
const featuredMediaUrl = featuredMedia?.url || '';
const publisherLogoUrl = publisherLogo?.url || '';

if (fonts && Object.keys(fonts).length >= 1) {
// if fonts are stored at the story level, populate the font data to the elements
pages = populateElementFontData(pages, fonts);
}

return (
<html amp="" lang="en">
<head>
Expand Down Expand Up @@ -100,6 +107,7 @@ function OutputStory({

OutputStory.propTypes = {
story: PropTypes.shape({
fonts: PropTypes.object,
link: PropTypes.string,
title: PropTypes.string.isRequired,
autoAdvance: PropTypes.bool,
Expand Down
1 change: 1 addition & 0 deletions packages/output/src/test/story.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ describe('Story output', () => {
width: 640,
height: 853,
},
fonts: {},
publisherLogo: {
id: 1,
url: 'https://example.com/logo.png',
Expand Down
35 changes: 35 additions & 0 deletions packages/output/src/utils/populateElementFontData.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

function populateElementFontProperties({ elements, ...rest }, fonts) {
if (!fonts) {
return { elements, ...rest };
}

return {
elements: elements.map((element) => {
if (element.type === 'text' && Boolean(element.font?.family)) {
return { ...element, font: fonts[element.font?.family] };
}
return element;
}),
...rest,
};
}

export function populateElementFontData(data, fonts) {
return data.map((page) => populateElementFontProperties(page, fonts));
}
1 change: 1 addition & 0 deletions packages/output/src/utils/test/getStoryMarkup.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ describe('getStoryMarkup', () => {
width: 640,
height: 853,
},
fonts: {},
password: '',
};
const metadata = {
Expand Down
61 changes: 61 additions & 0 deletions packages/output/src/utils/test/populateElementFontData.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* Internal dependencies
*/
import { populateElementFontData } from '../populateElementFontData';

describe('populateElementFontData', () => {
it('adds font properties to elements', () => {
const fonts = {
Oswald: {
family: 'Oswald',
service: 'fonts.google.com',
metrics: { upm: 1000, asc: 1050, des: -350 },
},
Roboto: {
family: 'Roboto',
service: 'fonts.google.com',
metrics: { upm: 1000, asc: 1048, des: -271 },
},
};

const pages = [
{
elements: [
{ type: 'text', font: { family: 'Oswald' } },
{ type: 'shape' },
],
},
{
elements: [
{ type: 'shape' },
{ type: 'text', font: { family: 'Roboto' } },
],
},
];

const newPages = populateElementFontData(pages, fonts);
expect(newPages[0].elements[0]).toStrictEqual({
type: 'text',
font: { ...fonts['Oswald'] },
});
expect(newPages[1].elements[1]).toStrictEqual({
type: 'text',
font: { ...fonts['Roboto'] },
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ describe('useAutoSave', () => {
height: 100,
width: 100,
},
fonts: {},
password: '',
globalStoryStyles: '',
taxonomies: [],
Expand Down Expand Up @@ -155,6 +156,7 @@ describe('useAutoSave', () => {
height: 100,
width: 100,
},
fonts: {},
password: '',
globalStoryStyles: '',
taxonomies: [],
Expand Down
10 changes: 9 additions & 1 deletion packages/story-editor/src/app/story/effects/useLoadStory.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { useEffect } from '@googleforcreators/react';
import { migrate } from '@googleforcreators/migration';
import { createPage } from '@googleforcreators/elements';
import { populateElementFontData } from '@googleforcreators/output';

/**
* Internal dependencies
Expand Down Expand Up @@ -82,7 +83,13 @@ function loadStory(storyId, post, restore, clearHistory, globalConfig) {
// If there are no pages, create empty page.
const storyData =
storyDataRaw && migrate(storyDataRaw, storyDataRaw.version || 0);
const pages = storyData?.pages?.length > 0 ? storyData.pages : [createPage()];
let pages = storyData?.pages?.length > 0 ? storyData.pages : [createPage()];

if (storyData?.fonts && Object.keys(storyData?.fonts).length >= 1) {
// fonts are stored in storyData,
// so we need to populate the font data back to the elements
pages = populateElementFontData(pages, storyData?.fonts);
timarney marked this conversation as resolved.
Show resolved Hide resolved
}

// Initialize color/style presets, if missing.
// Otherwise ensure the saved presets are unique.
Expand All @@ -99,6 +106,7 @@ function loadStory(storyId, post, restore, clearHistory, globalConfig) {
const story = {
storyId: storyId,
title,
fonts: storyData?.fonts || {},
status,
author,
date,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('addElementsAcrossPages', () => {
],
current: '111',
selection: ['000'],
story: { fonts: {} },
});

const result = addElementsAcrossPages({
Expand Down Expand Up @@ -98,7 +99,7 @@ describe('addElementsAcrossPages', () => {
animationState: STORY_ANIMATION_STATE.RESET,
capabilities: {},
copiedElementState: {},
story: {},
story: { fonts: {} },
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('restore', () => {
pages: [],
selection: [],
current: null,
story: {},
story: { fonts: {} },
});
});

Expand All @@ -125,7 +125,7 @@ describe('restore', () => {
pages: [],
selection: [],
current: null,
story: {},
story: { fonts: {} },
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ describe('updateStory', () => {

const result = updateStory({ properties: { a: 1, b: 2 } });

expect(result.story).toStrictEqual({ a: 1, b: 2 });
expect(result.story).toStrictEqual({ a: 1, b: 2, fonts: {} });
});

it('should update story with property updater function which will overwrite existing values', () => {
const { updateStory } = setupReducer();

const firstResult = updateStory({ properties: { a: 1, b: 2 } });
expect(firstResult.story).toStrictEqual({ a: 1, b: 2 });
expect(firstResult.story).toStrictEqual({ a: 1, b: 2, fonts: {} });

const secondResult = updateStory({
properties: (old) => ({ b: old.b + 2 }),
Expand All @@ -45,9 +45,9 @@ describe('updateStory', () => {
const { updateStory } = setupReducer();

const firstResult = updateStory({ properties: { a: 1, b: 2 } });
expect(firstResult.story).toStrictEqual({ a: 1, b: 2 });
expect(firstResult.story).toStrictEqual({ a: 1, b: 2, fonts: {} });

const secondResult = updateStory({ properties: { b: 3, c: 4 } });
expect(secondResult.story).toStrictEqual({ a: 1, b: 3, c: 4 });
expect(secondResult.story).toStrictEqual({ a: 1, b: 3, c: 4, fonts: {} });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const INITIAL_STATE = {
capabilities: {},
current: null,
selection: [],
story: {},
story: { fonts: {} },
animationState: STORY_ANIMATION_STATE.RESET,
copiedElementState: {},
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

function reduceElementFontProperties({ elements, ...rest }) {
return {
elements: elements.map((element) => {
if (element.type === 'text' && Boolean(element.font?.family)) {
return { ...element, font: { family: element.font.family } };
}
return element;
}),
...rest,
};
}

export function cleanElementFontProperties(data) {
return data.map((page) => reduceElementFontProperties(page));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
export function getStoryFontsFromPages(pages) {
const fonts = {};
pages.forEach(({ elements = [] }) =>
elements.forEach((element) => {
if (element.type === 'text' && Boolean(element.font?.family)) {
fonts[element.font.family] = element.font;
}
})
);
return fonts;
}
25 changes: 21 additions & 4 deletions packages/story-editor/src/app/story/utils/getStoryPropsToSave.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ import { getStoryMarkup } from '@googleforcreators/output';
*/
import objectPick from '../../../utils/objectPick';
import getAllProducts from './getAllProducts';
import { cleanElementFontProperties } from './cleanElementFontProperties';
import { getStoryFontsFromPages } from './getStoryFontsFromPages';

function getStoryPropsToSave({ story, pages, metadata, flags }) {
const { terms, ...propsFromStory } = objectPick(story, [
const { terms, fonts, ...propsFromStory } = objectPick(story, [
'title',
'fonts',
'status',
'author',
'date',
Expand All @@ -43,12 +46,26 @@ function getStoryPropsToSave({ story, pages, metadata, flags }) {
'backgroundAudio',
'terms',
]);
const products = getAllProducts(pages);
const content = getStoryMarkup(story, pages, metadata, flags);

// clean up fonts to store at the story level
// this avoids storing the same font (properties) multiple times
// see: https://github.com/GoogleForCreators/web-stories-wp/issues/12261
const cleandFonts = getStoryFontsFromPages(pages);
// clean up text elements to remove font properties from individual elements
const cleanedPages = cleanElementFontProperties(pages);
timarney marked this conversation as resolved.
Show resolved Hide resolved
const products = getAllProducts(cleanedPages);
const content = getStoryMarkup(
{ ...story, fonts: cleandFonts },
cleanedPages,
metadata,
flags
);

return {
content,
pages,
pages: cleanedPages,
...propsFromStory,
fonts: cleandFonts,
...terms,
products,
};
Expand Down
Loading