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

Falling back to contentTypeName when Block List label is empty #10963

Merged

Conversation

callumbwhyte
Copy link
Contributor

@callumbwhyte callumbwhyte commented Aug 26, 2021

I'm a big fan of the Block List Editor! And especially love how powerful the label templating feature is, allowing us to give our editors more context. Where a block content has a title / heading property we usually opt for this as our label.

However, sometimes properties are optional... and therefore the label can be empty...

The way to combat this is to have a condition in the label template {{title || 'My Block Name'}} but this means hardcoding content type names...

This PR adds a null check around the result of the label interpolator, and if there is no output falls back to the default value: the block list content type name.

There are some cases where hardcoding the content type names is necessary, such as Block name: {{title}}. This PR also introduces a variable to easily access the name in label templates: $contentTypeName

@umbrabot
Copy link

umbrabot commented Aug 26, 2021

Hi there @callumbwhyte, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

  • It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
  • The automated tests all pass (see "Checks" tab on this PR)
  • The level of security for this contribution is the same or improved
  • The level of performance for this contribution is the same or improved
  • Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
  • If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
  • The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@FransdeJong
Copy link
Contributor

@callumbwhyte
I think I see a issue with this solution.

We name our blocks with the content block name and if the value we are looking for isn't empty we add "- block title"

For example: Text block - text block one.

The reason we do this is that the editor can always see the type of block and if the optional value is filled it is visible for them to.

I think with your solution what we will end up with is: Text block - Text block

@callumbwhyte
Copy link
Contributor Author

@FransdeJong Not sure I understand the issue...

If you have a value in the "label" field that returns something then that will be used, no changes there.

In your case, it sounds like you have a label template like this: {{blockTitle ? 'Block name - ' + blockTitle : 'Block name'}}?

This change won't affect your case. You could even adjust it to {{blockTitle ? 'Block name - ' + blockTitle : null}} if you wanted to avoid hardcoding the block name.

@FransdeJong
Copy link
Contributor

We had something like this: Block Name {title ? '- ' + title : ''}
But I can make it work with your example. I have to refactor the old projects when we update.

@callumbwhyte
Copy link
Contributor Author

In which case that should keep working! 😊 As your label will always have "Block Name" in it then that will always be returned.

Something I considered adding to this PR was a variable for the contentTypeName, e.g. $contentTypeName, which could be used in your case to avoid hardcoding the name... Thoughts?

@FransdeJong
Copy link
Contributor

In my case that would be brilliant. That way I can work with a default code in that field for every block.

I can look at it with you after I return from my holiday if you like

@callumbwhyte
Copy link
Contributor Author

@FransdeJong Consider it done, just added to this PR!

@nathanwoulfe
Copy link
Contributor

Looks good to me, have tested with the following (block has a property aliased featureName):

  • label: {{ featureName }} - {{ $contentTypeName }} => renders featureName value and content type name
  • label: {{ foo }} => renders content type name as no value exists for foo
  • label: {{ featureName }} - foo => renders featureName value and foo
  • label: empty => renders content type name
  • label per @FransdeJong example: Block Name {{ featureName ? '- ' + featureName : '' }} => renders correctly when featureName has a value, and renders Block Name when featureName is null

Unless I'm missing any other valid test cases, this is good to go.

@nathanwoulfe nathanwoulfe merged commit 405ed44 into umbraco:v8/contrib Sep 21, 2021
@callumbwhyte callumbwhyte deleted the block-list-label-defaults branch September 21, 2021 08:05
@callumbwhyte
Copy link
Contributor Author

Just wondering what the process is for also getting this merged up to V9? 🙏

@nul800sebastiaan
Copy link
Member

Everything in v8 will be merged up to v9 still for the foreseeable future. :-) This didn't make it for the v9 final though, that one is in sync with 8.17. So I think it will be 9.1.0.

bergmania added a commit that referenced this pull request Oct 20, 2021
* Adjust icon in umb-checkbox and ensure icon is centered

* Missing nl translation for blockEditor_addBlock

* Implement icon parameter for doctype editor (#11008)

* fix: implement icon parameter for doctype editor

issue #10108

* fix: move color from icon to class attribute

* fix: removed defined colors, defaulting to the standard dark grey (ie "no color picked" in icon picker)

* cleaned up unused dependencies, double quotes to single, removed unused 'color' param from the create methods, and use shorthand object creation in createDocType (if the key has the same name as the variable passed as a prop, we only need to pass the key name)

* fix comment

Co-authored-by: Nathan Woulfe <[email protected]>

* Align sortable handle vertically in multivalues prevalue editor

* 10341: Use different picker for content types (#10896)

* 10341: Use different picker for content types

* use es6 where possible (inc removing underscore for teeny tiny performance improvement)

Co-authored-by: Nathan Woulfe <[email protected]>

* Falling back to contentTypeName when Block List label is empty (#10963)

* Falling back to contentTypeName when Block List label is empty

* Adding $contentTypeName variable for Block List labels

* Fix incorrect attribute

* Grid: Add button styling fix (#10978)

* Add missing focus styling

* Ensure add button is perfectly rounded and remove unused / uneeded CSS.

* Remove redundant border-color property

* Revert removal of unused css

Co-authored-by: BatJan <[email protected]>
Co-authored-by: Jan Skovgaard Olsen <[email protected]>

* Create content template localization (#10945)

* Don't use self-closing element for custom HTML elements

* Use button element for close/cancel in copy dialog

* Update localization of "createBlueprintFrom"

Co-authored-by: Nathan Woulfe <[email protected]>

* Cleanup examine search results, and adds ability to toggle fields (#9141)

* Cleanup examine search results, and adds ability to toggle fields

* update table to use joinarray filter with one-time binding to avoid recalculating filter values, updated filter to not explode when array arg is null

* fix failing tests - improve filter to not fail on non-array params, update tests accordingly

Co-authored-by: Nathan Woulfe <[email protected]>

* Add EntityController GetUrlsByUdis

Enables loading multiple URLs in a single request for Media & Documents

* Update content picker to use GetUrlsByUdis

* Allows replacing MainDom with alternate DB

There are some cases where there is a complex hosting strategy and folks want a readonly database and are hosting on Azure. In that case, it is not entirely possible to have a readonly Umbraco database because SqlMainDom is required and part of that requirement is to have read/write access to the umbraco key value table.
This PR allows for the default MainDom to be replaced and to allow for an SqlMainDomLock to use an alternate connection string so that a separate read/write database can be used.

* Remove inherited property group id/key when local properties are added (#11231)

* Remove inherited property group id/key when local properties are added

* Rebind saved content type values

* Remove inherited from save group

* Rename parameter for clarity

* Removes annoying wait text, which causes layout jank

* v8: Backoffice Welsh language translation updates (#11240)

* Updated the Welsh language file to include newly added keys (based on the en us language file)

* Updated the searchInputDescription key

* Updated the endTitle key

* Use medium sized overlay

* Use umb-icon component for icons in content type groups and tabs

* fixes wrong reference to enterSubmitFolder method in ng-keydown

* 11251: Don't add default dashboard to url

* Fix preview of SVG when height and width not are set

* If caching a published document, make sure you use the published Name… (#11313)

* If caching a published document, make sure you use the published Name. Closes #11074.

* Fix case of new node

Co-authored-by: Moore, Douglas S <[email protected]>

* Added missing Italian translations (#11197)

* Resolve incorrect ContentSavedState for failed publish

Closes #11290 (for v8)

* add modelValue validation for server to correctly update validation errors

* 11048: Bugfix for groups and properties that get replaced (#11257)

(cherry picked from commit 1605dc1)

* Icon fallback to `icon-document` for existing document types (#11283)

* Align create buttons styling (#11352)

* Added button for cancelling dictionary create action

* Use hideMenu

* Align dictionary create with the other creates

* Align import documenttype

* Align for data type folder create

* Align document type create buttons

* Forgot small ng-show

* Align create media folder buttons

* Align create macro buttons

* Align create relation buttons

* Align create partial view macro folder buttons

* Align partial view folder create buttons

* Align create scripts folder buttons

* Align create scripts folder buttons

* Use primary instead of success

* V8: Duplicate MemberGroup names cause MemberGroup mixup (#11291)

* Prevented duplicate member group names

* Added English lang

* Updated 'Exist' typo

* add labels in FR and NL

* Adding property group aliases to ex.message

* Adding invalid prop group aliases as ModelState errors, so we don't introduce breaking changes

* Pointing the actual reason for invalidating composition

* Validate all content type dependencies and throw a single InvalidCompositionException

* Rename based on review comments

* Update composition validation error messages

* Update InvalidCompositionException message

* Allow switching property editor from numeric to slider (#11287)

* Make it possible to change from numeric/decimal property editor to slider without breaking editor

* Formatting

* Enables friendly pasting in multipletextbox

* UI API docs: Added reset rules for .close class

* UI API docs: Fixed incorrect method name

* 11331: Check property on instance if id is not set yet

* Fixed cypress tests

Co-authored-by: Bjarne Fyrstenborg <[email protected]>
Co-authored-by: Corné Strijkert <[email protected]>
Co-authored-by: Søren Gregersen <[email protected]>
Co-authored-by: Nathan Woulfe <[email protected]>
Co-authored-by: patrickdemooij9 <[email protected]>
Co-authored-by: Callum Whyte <[email protected]>
Co-authored-by: Jan Skovgaard <[email protected]>
Co-authored-by: BatJan <[email protected]>
Co-authored-by: Jan Skovgaard Olsen <[email protected]>
Co-authored-by: Søren Kottal <[email protected]>
Co-authored-by: Paul Johnson <[email protected]>
Co-authored-by: Shannon <[email protected]>
Co-authored-by: Sebastiaan Janssen <[email protected]>
Co-authored-by: Ronald Barendse <[email protected]>
Co-authored-by: Owain Jones <[email protected]>
Co-authored-by: Mole <[email protected]>
Co-authored-by: Doug Moore <[email protected]>
Co-authored-by: Moore, Douglas S <[email protected]>
Co-authored-by: Martino Gabrielli <[email protected]>
Co-authored-by: Nikolaj Geisle <[email protected]>
Co-authored-by: Mads Rasmussen <[email protected]>
Co-authored-by: Jamie Townsend <[email protected]>
Co-authored-by: Elitsa Marinovska <[email protected]>
Co-authored-by: Anders Bjerner <[email protected]>
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.

5 participants