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 Show CMS field (top-level) tabs in more actions dropdown #373

Merged

Conversation

ScopeyNZ
Copy link
Contributor

@ScopeyNZ ScopeyNZ commented Sep 4, 2018

This introduces tab names to the "more actions" dropdown on individual elements in the elemental editor. This includes:

  • Server-side caching of tab names per element type
  • A new type for "element type" - If there's suggestions for better naming I'm all ears

Resolves #323

Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

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

Nice overall, some minor comments

src/Forms/ElementalAreaField.php Outdated Show resolved Hide resolved
src/Services/ElementTabProvider.php Outdated Show resolved Hide resolved
src/Services/ElementTabProvider.php Outdated Show resolved Hide resolved
src/Services/ElementTabProvider.php Outdated Show resolved Hide resolved
src/Services/ElementTabProvider.php Outdated Show resolved Hide resolved
src/Services/ElementTabProvider.php Outdated Show resolved Hide resolved
@ScopeyNZ ScopeyNZ force-pushed the pulls/4/edit-tab-le branch from f116276 to c0fd57b Compare September 5, 2018 00:37
@ScopeyNZ
Copy link
Contributor Author

ScopeyNZ commented Sep 5, 2018

Updated from feedback 🙂

Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

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

Minor stuff now - comments mainly

client/src/components/ElementEditor/Header.js Outdated Show resolved Hide resolved
@ScopeyNZ ScopeyNZ force-pushed the pulls/4/edit-tab-le branch 2 times, most recently from df37c9c to 55411b2 Compare September 6, 2018 00:13
@ScopeyNZ
Copy link
Contributor Author

ScopeyNZ commented Sep 6, 2018

Updated again and rebased.

@ScopeyNZ ScopeyNZ force-pushed the pulls/4/edit-tab-le branch from 55411b2 to ba53b3a Compare September 6, 2018 00:17
Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

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

Nice, I'm just going to test this out locally and will push a commit for my very minor suggestion in the config

namespace: "ElementTabCache"
DNADesign\Elemental\Services\ElementTabProvider:
properties:
cache: %$Psr\SimpleCache\CacheInterface.ElementTabCache
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's always safest to quote injector definitions like this, I've had problems without quoting them before. Also we could put these config definitions into a new cache.yml file

Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

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

Beauty. It's nice to see that the cache generation doesn't take very long either.

@robbieaverill robbieaverill merged commit 855c81a into silverstripe:master Sep 6, 2018
@robbieaverill robbieaverill deleted the pulls/4/edit-tab-le branch September 6, 2018 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants