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

[Fix] Block editor labels showing Angular JS on first load. #14143

Merged
merged 3 commits into from May 19, 2023

Conversation

dKumarmj
Copy link
Contributor

Prerequisites

The Block Label of Block Components is displaying Angular JS code rather than result on first load. If opened again the label displays correctly

Reported issue #14105

Description

old code

blockObject.label = (blockObject.config.label || blockObject.content?.contentTypeName) ?? "" ;

new code

blockObject.label = (blockObject.labelInterpolator(blockObject.data) || blockObject.content?.contentTypeName) ?? "";

Replace blockObject.config.label with blockObject.labelInterpolator(blockObject.data), because of blockObject.config.label always has the property alias eg: {{propertyAlias}}

@github-actions
Copy link

Hi there @dKumarmj, 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 🤖 🙂

@nul800sebastiaan
Copy link
Member

Thanks @dKumarmj, I think ng-bind-html is a great alternative, especially since the goal was not to do anything very special at all, the goal was:

Assigns the config label as the first instance, if this is not set (left empty) then uses the contenttype name of the block. If that fails, which is unlikely, will default to an empty string to prevent failure elsewhere.

I'd love to give it another test after you make those changes, should be quick to merge then! 👍

@iOvergaard
Copy link
Contributor

iOvergaard commented May 10, 2023

@nul800sebastiaan > I think that ng-bind-html is not a good solution, since people expect to be able to use filters for this label, for example, the infamous ncNodeName filter is a good one which needs to work unfortunately.

The latest change on this PR effectually just reverts the change made in the former PR #13937 that broke it in the first place. Personally, I think this is the best we can do, since a computed label does not make sense before you enter any value in the item anyway, so I would recommend that we simply merge this current PR and pick it for v10 and v11.

What do you think?

P.S. We'll try and see how we can improve on this in the new backoffice where we are not subject to the limitations of AngularJS's interpolator 😢

@nul800sebastiaan nul800sebastiaan merged commit 58695b6 into umbraco:contrib May 19, 2023
13 checks passed
@nul800sebastiaan
Copy link
Member

@iOvergaard Sounds good to me! That said, I can't for the life of me reproduce the originally reported behavior reported so I am not sure if maybe something else was also fixed along the way. I'll comment on the other issue as well.

Thanks very much @dKumarmj for the help and your flexibility here! #h5yr 👍

nul800sebastiaan pushed a commit that referenced this pull request May 19, 2023
Co-authored-by: Dhanesh Kumar <“dhanesh.kumar@phases.io”>
(cherry picked from commit 58695b6)
nul800sebastiaan pushed a commit that referenced this pull request May 19, 2023
Co-authored-by: Dhanesh Kumar <“dhanesh.kumar@phases.io”>
(cherry picked from commit 58695b6)
@nul800sebastiaan
Copy link
Member

Cherry picked for v10 in 16f448a
Cherry picked for v11 in 7d3c22b

@marcloveUSN
Copy link

Hi, this issue should be re-opened. This is still occurring in Umbraco 11.4.0

@Chris-Barber
Copy link

This is happening in Umbraco Cloud 10.5.1

@iOvergaard
Copy link
Contributor

@marcloveUSN I just had a look around. Unfortunately, it seems something has gone wrong with the merge and this fix was never included in 11.4.0. I have retagged this to 11.4.1, which is slated for release later this week.

We are very sorry about the confusion and frustration this might have caused.

@iOvergaard
Copy link
Contributor

@Chris-Barber Thanks for testing this. The fix for the version 10 series of Umbraco will be available with version 10.6.0 which is slated for release in the beginning of July.

@marcloveUSN
Copy link

@iOvergaard thanks for the update. Glad to hear its now resolved.

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.

None yet

6 participants