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.

Sign up for GitHub

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

Allow user to edit file information directly in the modal #1038

Merged
merged 6 commits into from
May 4, 2020
Merged

Allow user to edit file information directly in the modal #1038

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bc3f4a3
NEW Add ability to edit file in file insert modals
Dec 18, 2019
cd08e1f
API Bump @silverstripe/webpack-config to 1.5.0
Apr 27, 2020
55d740e
Adjustement following peer review feedback.
Apr 28, 2020
9c00030
Rename the asset specific Breadcrumb to AssetAdminBreadcrumb
Apr 28, 2020
0829a28
Target a specific version of framewortest
Apr 29, 2020
d6f1f2c
Only call set readonly if we actually need to set it
May 1, 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
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ before_script:
# Install composer
- composer validate
- if [[ $DB == PGSQL ]]; then composer require --no-update silverstripe/postgresql:2.x-dev --prefer-dist; fi
- composer require silverstripe/recipe-testing:^1 silverstripe/recipe-cms 4.x-dev --no-update --prefer-dist
- composer require silverstripe/recipe-testing:^1 silverstripe/recipe-cms:4.x-dev silverstripe/frameworktest:dev-master --no-update --prefer-dist
Copy link
Member

@emteknetnz emteknetnz Apr 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

As discussed on slack:

  • including frameworktest as dev-dependency in composer WILL NOT require frameworktest unless it's the in the project root.
  • means it won't do anything on your local if you're doing development from the vendor folder (as most of us are)
  • in the case of asset-admin being the root project, which it will in travis, it is functionally the same, though it's clearer being in travis.yml because it shows the intent better

Seems like the bigger question though is why is frameworktest a requirement for behat tests in the first place? I've always seen frameworktest as a quick and dirty way to create lots of records for manual testing. We're including dev-master here because we don't bother versioning frameworktest

Rather then rely on frameworktest for behat fixture creation, seems like it would be much cleaner to create them the old fashioned way i.e. yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a precedent for using frameworktests ... it gets installed by the CWP kitchen sink.

I can't just rely on creating fixtures with YML because I need an DataObject with an has_many relationship to Image or File. Off the top of my head, I don't think we have a recipe that has that particular set up out of the box

Copy link
Member

@emteknetnz emteknetnz Apr 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

You can probably guess what I'm going to say next :-)

Just create a TestOnly DataObject in asset-admin and use that. Much cleaner and safer than using frameworktest that has no versioning and could change at any point.

Also means you can run behat tests locally without having frameworktest installed (and which isn't specified any where but in .travis.yml that it's a requirement)

- composer update

# Remove preinstalled Chrome (google-chrome)
Expand Down
2 changes: 1 addition & 1 deletion client/dist/js/TinyMCE_ssembed.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/dist/js/TinyMCE_sslink-file.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/dist/js/TinyMCE_ssmedia.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/dist/js/bundle.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/dist/styles/bundle.css
View file
Open in desktop

Large diffs are not rendered by default.

21 changes: 12 additions & 9 deletions client/lang/src/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
{
"AssetAdmin.ADD_FILES": "Add from files",
"AssetAdmin.ADD_FOLDER_BUTTON": "Add folder",
"AssetAdmin.BACK": "Back",
"AssetAdmin.BACK_DESCRIPTION": "Navigate up a level",
"AssetAdmin.BROWSE": "Browse",
"AssetAdmin.BULK_ACTIONS_CONFIRM": "Are you sure you want to %s these files?",
"AssetAdmin.BULK_ACTIONS_DELETE": "Delete",
"AssetAdmin.BULK_ACTIONS_DELETE_SINGLE_ITEM_CONFIRM": "Are you sure you want to delete this file/folder?",
Expand All @@ -18,7 +20,10 @@
"AssetAdmin.CANCEL": "Cancel",
"AssetAdmin.CONFIRMDELETE": "Are you sure you want to delete this record?",
"AssetAdmin.CREATED": "First uploaded",
"AssetAdmin.CreateTitle": "Insert new media from the web",
"AssetAdmin.DELETE": "Delete",
"AssetAdmin.DETAILS": "Details",
"AssetAdmin.DISMISS": "Dismiss",
"AssetAdmin.DIM": "Dimensions",
"AssetAdmin.DROPZONE_CANCEL_UPLOAD": "Cancel upload",
"AssetAdmin.DROPZONE_CANCEL_UPLOAD_CONFIRMATION": "Are you sure you want to cancel this upload?",
Expand All @@ -34,6 +39,9 @@
"AssetAdmin.DROPZONE_SUCCESS_UPLOAD": "File uploaded",
"AssetAdmin.DROPZONE_UPLOAD": "Upload",
"AssetAdmin.EDIT": "Edit",
"AssetAdmin.EditTitle": "Media from the web",
"AssetAdmin.EMPTY": "No files",
"AssetAdmin.ERROR_OEMBED_REMOTE": "Embed is only compatible with remote files",
"AssetAdmin.FILENAME": "Filename",
"AssetAdmin.FILES": "Files",
"AssetAdmin.FILE_MISSING": "File cannot be found",
Expand All @@ -45,7 +53,10 @@
"AssetAdmin.JOINLAST": "and",
"AssetAdmin.LASTEDIT": "Last changed",
"AssetAdmin.LOADMORE": "Load more",
"AssetAdmin.NEXT": "Next",
"AssetAdmin.NOITEMSFOUND": "No items found",
"AssetAdmin.OR": "or",
"AssetAdmin.PREVIOUS": "Previous",
"AssetAdmin.PROMPTFOLDERNAME": "Please enter a folder name (or blank to cancel)",
"AssetAdmin.REPlACE_FILE_SUCCESS": "Upload successful, the file will be replaced when you Save.",
"AssetAdmin.SAVE": "Save",
Expand All @@ -63,13 +74,5 @@
"AssetAdmin.TITLE": "Title",
"AssetAdmin.TYPE": "File type",
"AssetAdmin.URL": "URL",
"AssetAdmin.ADD_FILES": "Add from files",
"AssetAdmin.BROWSE": "Browse",
"AssetAdmin.EMPTY": "No files",
"AssetAdmin.OR": "or",
"AssetAdmin.ERROR_OEMBED_REMOTE": "Embed is only compatible with remote files",
"AssetAdmin.CreateTitle": "Insert new media from the web",
"AssetAdmin.EditTitle": "Media from the web",
"AssetAdmin.NEXT": "Next",
"AssetAdmin.PREVIOUS": "Previous"
"AssetAdmin.VIEW": "View",
}
4 changes: 2 additions & 2 deletions client/src/components/GalleryItem/GalleryItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,8 @@ class GalleryItem extends Component {
action = this.handleCancelUpload;
actionIcon = 'font-icon-cancel';
} else if (this.exists()) {
const label = i18n._t('AssetAdmin.DETAILS', 'Details');
overlay = <div className="gallery-item--overlay font-icon-edit">{label}</div>;
const label = i18n._t('AssetAdmin.VIEW', 'View');
overlay = <div className="gallery-item--overlay font-icon-eye">{label}</div>;
}

const badge = this.props.badge;
Expand Down
4 changes: 3 additions & 1 deletion client/src/components/GalleryItem/GalleryItem.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,9 @@ $gallery-item-label-height: 40px;
text-align: center;

&::before {
margin-right: 5px;
margin-right: .385rem;
position: relative;
top: .2rem;
}
}

Expand Down
5 changes: 5 additions & 0 deletions client/src/components/UploadField/UploadField.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import CONSTANTS from 'constants/index';
import fieldHolder from 'components/FieldHolder/FieldHolder';
import fileShape from 'lib/fileShape';
import * as uploadFieldActions from 'state/uploadField/UploadFieldActions';
import * as modalActions from 'state/modal/ModalActions';
import PropTypes from 'prop-types';

/**
Expand Down Expand Up @@ -233,6 +234,7 @@ class UploadField extends Component {
* @param {Object} selectingItem
*/
handleReplaceShow(event, selectingItem) {
this.props.actions.modal.initFormStack('select', 'admin');
this.setState({
selecting: true,
selectingItem,
Expand Down Expand Up @@ -272,6 +274,7 @@ class UploadField extends Component {
*/
handleAddShow(event) {
event.preventDefault();
this.props.actions.modal.initFormStack('select', 'admin');
this.setState({
selecting: true,
selectingItem: null,
Expand All @@ -282,6 +285,7 @@ class UploadField extends Component {
* Close 'add from files' dialog
*/
handleHide() {
this.props.actions.modal.reset();
this.setState({
selecting: false,
selectingItem: null,
Expand Down Expand Up @@ -587,6 +591,7 @@ function mapDispatchToProps(dispatch) {
return {
actions: {
uploadField: bindActionCreators(uploadFieldActions, dispatch),
modal: bindActionCreators(modalActions, dispatch)
},
};
}
Expand Down
4 changes: 4 additions & 0 deletions client/src/components/UploadField/tests/UploadField-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ describe('UploadField', () => {
setFiles: jest.fn(),
removeFile: jest.fn(),
},
modal: {
initFormStack: jest.fn(),
reset: jest.fn(),
}
},
data: {
multi: true,
Expand Down
Loading