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

NEW Add content for 'Add Block' popover widget #371

Conversation

raissanorth
Copy link
Contributor

This is the ground work for adding a popover to add a new 'Add Block' widget for the Elemental Editor. There is yet some work remaining, as decisions from the Style Council are still outstanding - see #370

There are also styling issues with Edge and IE, but these can be addressed as part of the ongoing work related to this feature:
image

The buttons are keyboard accessible on the latest versions of Edge, IE, Firefox, Chrome, and Safari.

Note that this does not return a popover itself, but the content that could be rendered within a Popover component.

this.handleLinkClick = this.handleLinkClick.bind(this);
}

handleLinkClick() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This was removed right? There's still a few references to this stuff. Sad to see it go 😢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here.


AddElementPopoverContent.defaultProps = {
elementTypes: [
{ title: 'banner', icon: 'font-icon-block-banner' },
Copy link
Contributor

Choose a reason for hiding this comment

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

The default props should not have test data in it any more

@raissanorth raissanorth force-pushed the pulls/master/add-add-element-widget branch from 7c6e2f0 to 9844855 Compare September 4, 2018 01:45

return elementTypes.map((elementType) =>
(
<Button className={
Copy link
Contributor

Choose a reason for hiding this comment

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

You will need to add a key= in here. There is an identifier (of sorts) on the elementType you can use.


render() {
return (
<div className="element-editor__add-element-popover-content">
Copy link
Contributor

Choose a reason for hiding this comment

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

You're going to hate me for this one. I think we should refactor these class names. I'm terrible at this myself because BEM confuses me. It's supposed to be like this:

.block__element--modifier

Modifiers should only be things like --hidden or --disabled

We should probably refactor this so element-editor-add-element is the "block" and content is the "element". Ie. className="element-editor-add-element__content". And then for the next div className="element-editor-add-element__button-container".

@raissanorth raissanorth force-pushed the pulls/master/add-add-element-widget branch from 9844855 to 202ab8a Compare September 4, 2018 03:37
@ScopeyNZ ScopeyNZ merged commit e17ccfd into silverstripe:master Sep 4, 2018
@ScopeyNZ ScopeyNZ deleted the pulls/master/add-add-element-widget branch September 4, 2018 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants