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

Spritelab/Gamelab Animation Library changes for backgrounds #36578

Merged
merged 17 commits into from
Sep 25, 2020
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/i18n/common/en_us.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"animationCategory_tools": "Tools",
"animationCategory_vehicles": "Vehicles",
"animationMode": "Animation",
"animationPicker_allCategories": "All categories",
"animationPicker_drawYourOwn": "Draw your own",
"animationPicker_error": "Error: {message}",
"animationPicker_failedToParseImage": "The image could not be parsed",
Expand Down
6 changes: 6 additions & 0 deletions apps/src/code-studio/assets/searchAssets.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,9 @@ export function searchAssets(
)
};
}

export function filterOutBackgrounds(assets) {
return assets.filter(
animation => !animation.categories.includes('backgrounds')
);
}
8 changes: 8 additions & 0 deletions apps/src/p5lab/AnimationPicker/AnimationPicker.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ class AnimationPicker extends React.Component {
libraryManifest: PropTypes.object.isRequired,
hideUploadOption: PropTypes.bool.isRequired,
hideAnimationNames: PropTypes.bool.isRequired,
navigable: PropTypes.bool.isRequired,
defaultQuery: PropTypes.object,
hideBackgrounds: PropTypes.bool.isRequired,
canDraw: PropTypes.bool.isRequired,

// Provided via Redux
visible: PropTypes.bool.isRequired,
Expand Down Expand Up @@ -77,6 +81,10 @@ class AnimationPicker extends React.Component {
libraryManifest={this.props.libraryManifest}
hideUploadOption={this.props.hideUploadOption}
hideAnimationNames={this.props.hideAnimationNames}
navigable={this.props.navigable}
defaultQuery={this.props.defaultQuery}
hideBackgrounds={this.props.hideBackgrounds}
canDraw={this.props.canDraw}
/>
);
}
Expand Down
69 changes: 54 additions & 15 deletions apps/src/p5lab/AnimationPicker/AnimationPickerBody.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import ScrollableList from '../AnimationTab/ScrollableList.jsx';
import styles from './styles';
import AnimationPickerListItem from './AnimationPickerListItem.jsx';
import SearchBar from '@cdo/apps/templates/SearchBar';
import {searchAssets} from '@cdo/apps/code-studio/assets/searchAssets';
import {
searchAssets,
filterOutBackgrounds
} from '@cdo/apps/code-studio/assets/searchAssets';

const MAX_SEARCH_RESULTS = 40;
const MAX_HEIGHT = 460;
Expand Down Expand Up @@ -46,7 +49,11 @@ export default class AnimationPickerBody extends React.Component {
playAnimations: PropTypes.bool.isRequired,
libraryManifest: PropTypes.object.isRequired,
hideUploadOption: PropTypes.bool.isRequired,
hideAnimationNames: PropTypes.bool.isRequired
hideAnimationNames: PropTypes.bool.isRequired,
navigable: PropTypes.bool.isRequired,
defaultQuery: PropTypes.object,
hideBackgrounds: PropTypes.bool.isRequired,
canDraw: PropTypes.bool.isRequired
};

state = {
Expand All @@ -55,6 +62,26 @@ export default class AnimationPickerBody extends React.Component {
currentPage: 0
};

componentWillReceiveProps(nextProps) {
if (this.props.defaultQuery !== nextProps.defaultQuery) {
const currentPage = 0;
const {results, pageCount} = this.searchAssetsWrapper(
currentPage,
nextProps.defaultQuery
);
let nextQuery = nextProps.defaultQuery || {
categoryQuery: '',
searchQuery: ''
};
this.setState({
...nextQuery,
currentPage,
results,
pageCount
});
}
}

searchAssetsWrapper = (page, config = {}) => {
let {searchQuery, categoryQuery, libraryManifest} = config;

Expand Down Expand Up @@ -86,9 +113,10 @@ export default class AnimationPickerBody extends React.Component {
scrollWindow.scrollTop + MAX_HEIGHT >= scrollWindow.scrollHeight * 0.9 &&
(!pageCount || nextPage <= pageCount)
) {
const {results: newResults, pageCount} = this.searchAssetsWrapper(
nextPage
);
let {results: newResults, pageCount} = this.searchAssetsWrapper(nextPage);
if (this.props.hideBackgrounds) {
newResults = filterOutBackgrounds(newResults);
}

this.setState({
results: [...(results || []), ...newResults],
Expand All @@ -100,18 +128,24 @@ export default class AnimationPickerBody extends React.Component {

onSearchQueryChange = searchQuery => {
const currentPage = 0;
const {results, pageCount} = this.searchAssetsWrapper(currentPage, {
let {results, pageCount} = this.searchAssetsWrapper(currentPage, {
searchQuery
});
if (this.props.hideBackgrounds) {
results = filterOutBackgrounds(results);
}
this.setState({searchQuery, currentPage, results, pageCount});
};

onCategoryChange = event => {
const categoryQuery = event.target.className;
const currentPage = 0;
const {results, pageCount} = this.searchAssetsWrapper(currentPage, {
let {results, pageCount} = this.searchAssetsWrapper(currentPage, {
categoryQuery
});
if (this.props.hideBackgrounds) {
results = filterOutBackgrounds(results);
}
this.setState({categoryQuery, currentPage, results, pageCount});
};

Expand All @@ -126,7 +160,10 @@ export default class AnimationPickerBody extends React.Component {
};

animationCategoriesRendering() {
const categories = Object.keys(this.props.libraryManifest.categories || []);
let categories = Object.keys(this.props.libraryManifest.categories || []);
if (this.props.hideBackgrounds) {
categories = categories.filter(category => category !== 'backgrounds');
}
categories.push('all');
return categories.map(category => (
<AnimationPickerListItem
Expand Down Expand Up @@ -180,12 +217,14 @@ export default class AnimationPickerBody extends React.Component {
<div style={animationPickerStyles.navigation}>
{categoryQuery !== '' && (
<div style={animationPickerStyles.breadCrumbs}>
<span
onClick={this.onClearCategories}
style={animationPickerStyles.allAnimations}
>
{'All categories > '}
</span>
{this.props.navigable && (
<span
onClick={this.onClearCategories}
style={animationPickerStyles.allAnimations}
>
{`${msg.animationPicker_allCategories()} > `}
Copy link
Contributor

Choose a reason for hiding this comment

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

we should move the carrot into the animationPicker_allCategories string so that it displays correctly in RTL languages (right now, the greater-than sign would be on the incorrect end of the string)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Elijah asked me to take it out, do you still want me to put it back in? #36578 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, sorry, i missed his comment -- he's our resident i18n expert, so i'll take his recommendation 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

The browser is (usually) capable of aligning things like this correctly in RTL, and this way we avoid confusing translators with non-language characters

</span>
)}
<span>{msg[`animationCategory_${categoryQuery}`]()}</span>
</div>
)}
Expand All @@ -203,7 +242,7 @@ export default class AnimationPickerBody extends React.Component {
</div>
)}
{((searchQuery === '' && categoryQuery === '') ||
results.length === 0) && (
(results.length === 0 && this.props.canDraw)) && (
<div>
<AnimationPickerListItem
label={msg.animationPicker_drawYourOwn()}
Expand Down
35 changes: 30 additions & 5 deletions apps/src/p5lab/AnimationPicker/animationPickerModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import firehoseClient from '@cdo/apps/lib/util/firehose';
export const Goal = makeEnum('NEW_ANIMATION', 'NEW_FRAME');

const SHOW = 'AnimationPicker/SHOW';
const SHOW_BACKGROUND = 'AnimationPicker/SHOW_BACKGROUND';
const HIDE = 'AnimationPicker/HIDE';
const BEGIN_UPLOAD = 'AnimationPicker/BEGIN_UPLOAD';
const HANDLE_UPLOAD_ERROR = 'AnimationPicker/HANDLE_UPLOAD_ERROR';
Expand All @@ -32,7 +33,9 @@ const initialState = {
goal: null,
uploadInProgress: false,
uploadFilename: null,
uploadError: null
uploadError: null,
isSpriteLab: false,
isBackground: false
};

export default function reducer(state, action) {
Expand All @@ -42,7 +45,20 @@ export default function reducer(state, action) {
if (!state.visible) {
return _.assign({}, initialState, {
visible: true,
goal: action.goal
goal: action.goal,
isBackground: false,
isSpriteLab: action.isSpriteLab
});
}
return state;

case SHOW_BACKGROUND:
if (!state.visible) {
return _.assign({}, initialState, {
visible: true,
goal: action.goal,
isBackground: true,
isSpriteLab: true
});
}
return state;
Expand Down Expand Up @@ -74,11 +90,18 @@ export default function reducer(state, action) {
* @returns {{type: string, goal: AnimationPicker.Goal }}
* @throws {TypeError} if a valid goal is not provided
*/
export function show(goal) {
export function show(goal, isSpriteLab) {
if ([Goal.NEW_ANIMATION, Goal.NEW_FRAME].indexOf(goal) === -1) {
throw new TypeError('Must provide a valid goal');
}
return {type: SHOW, goal: goal};
return {type: SHOW, goal: goal, isSpriteLab: isSpriteLab};
}

export function showBackground(goal) {
if (goal !== Goal.NEW_ANIMATION) {
throw new TypeError('Must provide a valid goal');
}
return {type: SHOW_BACKGROUND, goal: goal};
}

/**
Expand Down Expand Up @@ -212,7 +235,9 @@ export function pickLibraryAnimation(animation) {
return (dispatch, getState) => {
const goal = getState().animationPicker.goal;
if (goal === Goal.NEW_ANIMATION) {
dispatch(addLibraryAnimation(animation));
dispatch(
addLibraryAnimation(animation, getState().animationPicker.isSpriteLab)
);
} else if (goal === Goal.NEW_FRAME) {
dispatch(appendLibraryFrames(animation));
}
Expand Down
19 changes: 14 additions & 5 deletions apps/src/p5lab/AnimationTab/AnimationList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,30 @@ class AnimationList extends React.Component {
animationList: shapes.AnimationList.isRequired,
selectedAnimation: shapes.AnimationKey,
onNewItemClick: PropTypes.func.isRequired,
spriteLab: PropTypes.bool.isRequired
spriteLab: PropTypes.bool.isRequired,
hideBackgrounds: PropTypes.bool.isRequired
};

render() {
let addAnimation = (
<NewListItem
key="new_animation"
label={this.props.spriteLab ? i18n.newCostume() : i18n.newAnimation()}
onClick={this.props.onNewItemClick}
onClick={() => this.props.onNewItemClick(this.props.spriteLab)}
/>
);
let animationListKeys = this.props.animationList.orderedKeys;
if (this.props.hideBackgrounds) {
animationListKeys = animationListKeys.filter(key => {
return !(
this.props.animationList.propsByKey[key].categories || []
).includes('backgrounds');
});
}
return (
<ScrollableList style={styles.root} className="animationList">
{this.props.spriteLab && addAnimation}
{this.props.animationList.orderedKeys.map(key => (
{animationListKeys.map(key => (
<AnimationListItem
key={key}
animationKey={key}
Expand All @@ -66,8 +75,8 @@ export default connect(
spriteLab: state.pageConstants.isBlockly
}),
dispatch => ({
onNewItemClick() {
dispatch(show(Goal.NEW_ANIMATION));
onNewItemClick(isSpriteLab) {
dispatch(show(Goal.NEW_ANIMATION, isSpriteLab));
}
})
)(AnimationList);
6 changes: 5 additions & 1 deletion apps/src/p5lab/AnimationTab/AnimationTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class AnimationTab extends React.Component {
libraryManifest: PropTypes.object.isRequired,
hideUploadOption: PropTypes.bool.isRequired,
hideAnimationNames: PropTypes.bool.isRequired,
hideBackgrounds: PropTypes.bool.isRequired,

// Provided by Redux
columnSizes: PropTypes.arrayOf(PropTypes.number).isRequired,
Expand All @@ -85,7 +86,7 @@ class AnimationTab extends React.Component {
>
<div style={styles.animationsColumn}>
<P5LabVisualizationHeader />
<AnimationList />
<AnimationList hideBackgrounds={this.props.hideBackgrounds} />
</div>
<div style={styles.editorColumn}>
<PiskelEditor style={styles.piskelEl} />
Expand All @@ -103,6 +104,9 @@ class AnimationTab extends React.Component {
libraryManifest={this.props.libraryManifest}
hideUploadOption={this.props.hideUploadOption}
hideAnimationNames={this.props.hideAnimationNames}
navigable={true}
canDraw={true}
hideBackgrounds={this.props.hideBackgrounds}
/>
)}
</div>
Expand Down
11 changes: 9 additions & 2 deletions apps/src/p5lab/P5Lab.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,13 @@ P5Lab.prototype.init = function(config) {
startLibraries = JSON.parse(config.level.startLibraries);
}
project.sourceHandler.setInitialLibrariesList(startLibraries);
getStore().dispatch(setInitialAnimationList(this.startAnimations));
getStore().dispatch(
setInitialAnimationList(
this.startAnimations,
false /* shouldRunV3Migration */,
this.isSpritelab
)
);
this.studioApp_.resetButtonClick();
}.bind(this);

Expand Down Expand Up @@ -444,7 +450,8 @@ P5Lab.prototype.init = function(config) {
getStore().dispatch(
setInitialAnimationList(
initialAnimationList,
this.isSpritelab /* shouldRunV3Migration */
this.isSpritelab /* shouldRunV3Migration */,
this.isSpritelab
)
);

Expand Down