cofoundry icon indicating copy to clipboard operation
cofoundry copied to clipboard

Revise Page MetaData

Open HeyJoel opened this issue 8 years ago • 6 comments

The meta data part of pages has been carried forward from previous versions but isn't very relevant to SEO today or extensible.

  • ~~Description shouldn't be mandatory~~
  • ~~Keywords is irrelevant~~
  • Should include canonical url
  • Twitter cards, pinterest rich pins and other meta data is app specific and needs to be pluggable

HeyJoel avatar Nov 01 '16 12:11 HeyJoel

I've tidied things up a little solving these points:

  • Description shouldn't be mandatory
  • Keywords is irrelevant

I think we can defer the rest of this for a while, the biggest thing is being able to make pages more extensible with custom data which is a bigger job.

HeyJoel avatar Jan 31 '17 13:01 HeyJoel

On Custom Entity pages you can override the metadescription, but none of the other meta data. e.g. OpenGraph.

purplepiranha avatar Aug 24 '19 10:08 purplepiranha

@purplepiranha because custom entities aren't always attached to pages they do not include meta data, but you can easily add those properties to the custom entity data model yourself.

HeyJoel avatar Aug 27 '19 09:08 HeyJoel

Thanks Joel for your response.

I did some work on this yesterday and realised that for the majority of my custom entities it wouldn't be needed. In fact currently only the blog will.

I'm going down the route of having a couple of interfaces that can be implemented where needed:

public interface ICustomEntityPageWithOpenGraphDisplayModel
    {
        bool UsePageOpenGraphSettings { get; set; }

        OpenGraphData OpenGraph { get; set; }
    }
public interface ICustomEntityWithOpenGraphDataModel
    {
        [Display(Name = "Use OpenGraph settings specified at page level")]
        bool UsePageOpenGraphSettings { get; set; }

        [Display(Name = "OpenGraph Title")]
        string OpenGraphTitle { get; set; }

        [Display(Name = "OpenGraph Description")]
        string OpenGraphDescription { get; set; }

        [Image]
        [Display(Name = "OpenGraph Image")]
        int? OpenGraphImageAssetId { get; set; }
    }

And then using a ViewComponent to render the OpenGraph tags if appropriate:

public class OpenGraphViewComponent : ViewComponent
    {
        private readonly IContentRouteLibrary _contentRouteLibrary;
        private readonly ILogger _logger;

        public OpenGraphViewComponent(IContentRouteLibrary contentRouteLibrary, ILogger<OpenGraphViewComponent> logger)
        {
            _contentRouteLibrary = contentRouteLibrary;
            _logger = logger;
        }

        public IViewComponentResult Invoke(dynamic model)
        {
            try
            {
                return this.DoInvoke(model);
            }
            catch (Exception e)
            {
                // log
                Type type = model?.GetType();
                _logger.LogError(e, "Error rendering OpenGraph data for " + type.ToString());

#if DEBUG
                // throw for development only
                throw;
#endif
            }

            // if anything goes wrong then we fail gracefully by just removing this section from the page
            return Content(""); 
        }

        private IViewComponentResult DoInvoke(IPageViewModel model)
        {
            var url = _contentRouteLibrary.ToAbsolute(model.Page.PageRoute.FullPath);

            var viewModel = new OpenGraph()
            {
                Url = url,
                Title = model.Page.OpenGraph.Title,
                Description = model.Page.OpenGraph.Description,
                ImageAssetUrl = model.Page.OpenGraph.Image != null ? _contentRouteLibrary.ToAbsolute(_contentRouteLibrary.ImageAsset(model.Page.OpenGraph.Image)) : ""
            };

            return View(viewModel);
        }

        private IViewComponentResult DoInvoke<T>(ICustomEntityPageViewModel<T> model) where T : class, ICustomEntityPageDisplayModel
        {
            var viewModel = new OpenGraph();

            viewModel.Url = _contentRouteLibrary.ToAbsolute(model.CustomEntity.PageUrls.FirstOrDefault());

            if (model.CustomEntity.Model is ICustomEntityPageWithOpenGraphDisplayModel ogModel)
            {
                if (ogModel.OpenGraph != null && ogModel.UsePageOpenGraphSettings == false)
                {
                    viewModel.Title = ogModel.OpenGraph.Title;
                    viewModel.Description = ogModel.OpenGraph.Description;
                    viewModel.ImageAssetUrl = ogModel.OpenGraph.Image != null ? _contentRouteLibrary.ToAbsolute(_contentRouteLibrary.ImageAsset(ogModel.OpenGraph.Image)) : "";

                    return View(viewModel);
                }
            }

            viewModel.Title = model.Page.OpenGraph.Title;
            viewModel.Description = model.Page.OpenGraph.Description;
            viewModel.ImageAssetUrl = model.Page.OpenGraph.Image != null ? _contentRouteLibrary.ToAbsolute(_contentRouteLibrary.ImageAsset(model.Page.OpenGraph.Image)) : "";

            return View(viewModel);
        }

    }

purplepiranha avatar Aug 27 '19 13:08 purplepiranha

Sounds good. One thing I'd say is to avoid empty catch blocks - at the very least log the exception e.g. logger.LogError(e, "Error rendering component etc"), but also you might want to throw if in development mode to avoid missing errors.

HeyJoel avatar Aug 27 '19 14:08 HeyJoel

I agree. I have updated the above code in case anyone wants to use it.

purplepiranha avatar Aug 27 '19 18:08 purplepiranha