Skip to content

Instantly share code, notes, and snippets.

@BriceShatzer
Last active September 20, 2019 17:55
Show Gist options
  • Save BriceShatzer/bdd91a7d423d23b0f349b33ac34f9184 to your computer and use it in GitHub Desktop.
Save BriceShatzer/bdd91a7d423d23b0f349b33ac34f9184 to your computer and use it in GitHub Desktop.

Overview

Using the existing curation endpoint, we can save/load a CustomHeader item that will be used to store the attributes that can be consumed at page render to create the desired customized header look.

The CustomHeader item could look something like this:

{
    "itemType": "CustomHeader",
    "itemData": {
        "backgroud": {
            "type": "color",
            "value": "#509b22"
        },
        "primary": {
            "type": "text",
            "value": "This is a text headline"
            // possibly color? 
        },
        "secondary": {
            "type": "text",
            "value": "this is some paragraph text I suppose"
        },
        "tertiary": {
            "type": "links",
            "value": [
                {
                    "text": "Season 1",
                    "url": "/c/tv-shows/season-1"
                },
                {
                    "text": "Season 2",
                    "url": "/c/tv-shows/season-2"
                }
            ]
        }
    }
}

/*  potentially in the future we add additional content "types" and the types in each slot would be interchangeble.  
(ie:  "primary": {"type": "links", "value"...)
*/            

Time-frame

I feel like a basic MVP is achievable by 9/30. That basic MVP would allow for:

  • Custom background color
  • Non-formated text in both the primary (heading) & secondary (text) slots
  • Image in the primary (heading) slot
  • Ability to set the background to an image
  • Up to 4 links in the tertiary slot
  • overhang the curation module (see second image)
  • look like: Basic Wireframe

Once this is released, we can look to expand the functionality by order of importance and add things like:

  • Addition of a "Sponsor Badge" slot
  • Formatted and/or colored text in both the primary (heading) & secondary (text) slots

Ultimately arriving at something like this:
Full Wireframe

Pros/Cons

Pros

  • Custom header functionality can be added to any place a curation module currently lives.
  • Using the existing curation module endpoints means that new api/backend work should be required and also means that...
  • only a single call needs to be made to retrieve the data for both the curation module and the custom header, rather than 2 separate calls.
  • Potentially makes doing curation/header integrations easier in the future.

Cons

  • Every place the curation module possibly could live needs to be touched to ensure that it can properly handle the possibly of the new CustomHeader item type.
  • Making updates to either a custom header or a curation module on a page means that both parts need to be "Saved"
  • Custom header functionality can't easily live on pages were curation module can't/doesn't exist
    (ie: "Uncategorized" category/subcategory pages & permalink pages)
  • Potentially adds confusion/complexity around using the "Delete" endpoints as they would now remove both the data responsible for rendering the curation module and the custom header. Currently no client functionality exists utilizing this endpoint.

To Do

Rendering

Allow for a CustomHeader item

Make sure the page render and the curation module component can handle the new CustomHeader item. Update:
kinja-magma/client/controllers/categorization/category-page.js and/or
kinja-magma/utils/prepareCurationProps.js

Create CustomHeader Component

Build a custom component that consumes a CustomHeader item and renders it correctly on the page.

Editor

Update curation module with an "Empty" layout

To allow for instances were someone wants a custom header but no curation module.

This is option can be utilized via the "Remove curation module" button shown in this mock

in kinja-components/components/curation-layout/default-layouts-stub.js add to getDefaultLayouts

{
    group: 'None',
    cardinality: 1,
    zones: [
        {
            dimension: '1fr',
            numberOfItems: 0
        }
    ]
}

Add editor functionality

When the curation module edit "Pin" is clicked, the custom header component becomes editable.
On pages that don't currently have any custom header content, an empty editable header is displayed.

Update the curation module "Save" functionality so that it takes the values set in that editable header and bundles them into a CustomHeader item that is included in the existing curation save flow prior to POST.

"Background Color" would be set using a system color picker similar to special sections.

"Background Image" would be set via the same sort of functionality that is used on special sections.

"Links" would open a simple modal similar to what currently exists when adding a link on a special sections.

Fully completed editor mock

Notes/Links

In-vision Designs

{
"$schema": "http://json-schema.org/draft-07/schema#",
"title" : "Custom Header",
"description" : "Defines the structure of the data used to customizable the header of story type and category pages",
"type": "object",
"properties": {
"background": {
"type": "object",
"properties": {
"type": { "type": "string", "enum": ["color", "image"] }
},
"allOf":[
{
"if": { "properties": {"type": {"const":"color"}} },
"then": {"properties": {"value": {"$ref": "#color"}} }
},
{
"if": {"properties": {"type": {"const": "image"}} },
"then": {"properties": {"value": {"$ref": "#image"}} }
}
]
},
"primary": {
"type": "object",
"properties": {
"type": { "type": "string", "enum": ["plainText", "image"] }
},
"allOf":[
{
"if": { "properties": {"type": {"const":"plainText"}} },
"then": {"properties": {"value": {"type": "string"}} }
},
{
"if": {"properties": {"type": {"const": "image"}} },
"then": {"properties": {"value": {"$ref": "#image"}} }
}
]
},
"secondary": {
"type": "object",
"properties": {
"type": { "type": "string", "enum": ["plainText"] }
},
"allOf":[
{
"if": { "properties": {"type": {"const":"plainText"}} },
"then": {"properties": {"value": {"type": "string"}} }
}
]
},
"tertiary": {
"type": "object",
"properties": {
"type": { "type": "string", "enum": ["links"] },
"items": {
"type": "array",
"items": { "$ref": "#link" }
}
}
}
},
"typeDefinitions": {
"color": {
"$id": "#color",
"type": "string",
"pattern": "^#([a-f]|[A-F]|[0-9]){3}(([a-f]|[A-F]|[0-9]){3})?$"
},
"image": {
"$id": "#image",
"properties":{
"format" : {
"type":"string",
"enum": ["jpg" , "png" , "gif" , "bmp" , "svg"]
},
"id": { "type": "string" },
"altText": {"type": "string"}
},
"required":["format","id"]
},
"link": {
"$id": "#link",
"type": "object",
"properties": {
"text": { "type": "string"},
"url": { "type": "string", "pattern": "https?:\\\/\\\/[\\w\\-_]+(\\.[\\w\\-_]+)+([\\w\\-\\.,@?^=%&:\/~\\+#]*[\\w\\-\\@?^=%&\/~\\+#])?"}
},
"required":["text","url"]
}
}
}
@erniedeeb
Copy link

This is great, thank you Brice.

I think we may want to shuffle some things around.

MVP:
Custom background color
Non-formated text in both the primary (heading) & secondary (text) slots
Image in the primary (heading) slot
Ability to set the background to an image
Curation overhang

Unprioritized secondary items:
Addition of a "Sponsor Badge" slot
Formatted and/or colored text in both the primary (heading) & secondary (text) slots
Ability to set a background

What effect would moving curation overhang and the image additions have on your original estimate?

cc @borlaym

@borlaym
Copy link

borlaym commented Sep 12, 2019

Thanks Brice, this is great!
Regarding the coupling of the custom header to the curation. You raise excellent points in both the pro and cons sections. I’ld like to add a few:

  • Storing arbitrary JSON in the database is iffy at best, and it’s not a best practice, it shouldn’t have been done in the first place for the curation module. Storing structured data on the backend is usually preferred. If the CustomHeader would be part of the Curation Module, we would store arbitrary JSON.
  • Viktor is usually pretty light on tasks, not having to do backend work for a feature is not really a pro.
  • The CustomHeader data, if we’re sure that it’s only for storytype pages, and will never be needed for frontpages, could live with the storytype itself, so it wouldn’t require another call. This way we wouldn’t have to worry about saving the curation and customheader together and avoiding it on frontpages.

Really it’s up to whether we consider this feature part of the curation module or part of the story type pages. I’m leaning towards the latter, but it’s up to interpretation.

I see the edit functionality the most difficult part of this project. So the question is, is it set in stone that edit wants this feature? Is it 100% sure that we will ship this? If not, we could manually store some data in the db for some select storytypes and test it without having to implement the edit functionality first. Just an idea, but if we’re sure that this is requested by editors, then it’s fine to build them together.

@erniedeeb
Copy link

It's not only set in stone that edit wants this feature, they (as well as Jim) want it ASAP. Teams in both NY and CHI have already started designing assets for customization.

If the feature is well received on story type pages, and depending how the the homepage redesigns shake out, I could see a scenario where we're asked to apply the same treatment to homepages. So even if we build this as part of ST pages rather than the curation module itself, I'd like for us to be able to easily retrofit it to homepage curation if the need arises.

We can discuss more at stand up today, Etele will join and can perhaps provide some insight on the trajectory of homepage redesigns and the likelihood of using customization on homepages.

@vtanyi-gmg
Copy link

I think the pros/cons section is a great assessment! I'm a tiny bit wary of bundling the custom header into the curation module, because of the last 3 cons points and because the two modules seem somewhat independent to me conceptually (e.g. curation but no custom header, custom header without curation, multiple curations but only one custom header allowed).

But if we're okay with coupling the header and curation together, this looks like the most straightforward plan, especially if this is an ASAP project.

@erniedeeb
Copy link

@BriceShatzer answering your question from Slack here. Assuming making the overhang standard is less or an equal amount of work to a toggle, let's start there and think about adding a toggle later on. As for my MVP list, I think that's still up for discussion based on how much those changes add to scope.

Does "Non-formated text in both the primary (heading) & secondary (text) slots" include editing those fields from within the module? Because if so I think we can separate those things and move text editing out of MVP.

@borlaym
Copy link

borlaym commented Sep 13, 2019

One note. I think that it would make a bit more sense to elevate the customHeader data a level up in the JSON. If you check https://kinja.com/api/core/curation/views/forBlog?blogId=4 right now we have blogId, layout, items, updatedAt, updatedBy. Instead of adding the custamHeader data inside items, I think I'd rather have it on the same level as these keys I mentioned.
I'm not really familiar with the curation api, but it seems like inside items you only ever have the type of StoryCard, which are the actual items in the curation. It feels like the customHeader doesn't belong there.

On a side note, typing for the curation module is all kinds of messed up right now in our codebase.

@vtanyi-gmg
Copy link

That's a good idea, that involves a minor API change (adding the customHeader field) that can be done under a day.

If we go that way and the header data wouldn't be part of the already existing items field (which is just stored as the frontend sends it without any validation), the customHeader field could have a schema that the backend validates on saving. (Just a suggestion.)

@BriceShatzer
Copy link
Author

When playing around with the API originally, I tried including a CustomHeader attribute/object in the top level and found that I couldn't get it to save. Including it in the Items array was a work-around way to get everything saved without having to alter any of the GET/POST curation endpoints. Moving it up a level would most likely eliminate this task

@borlaym I agree that it would probably make more sense to live up a level.

Further, I agree with @vtanyi-gmg that having a defined schema for the structure of a "top-level" CustomHeader attribute/object that we can validates against would be ideal.

My only concern would be how difficult it would be to make changes and updates to something like that with a moderate moderate amount of complexity at a fairly rapid pace?
Once this is released I think any changes that are made will end up being relatively deliberate and iterative, but I suspect that might not be the case while we're building this out.

For example:
Despite laying out a fairly rudimentary structure in the document above,
I don't think I would be in a place to tell you how image info should be stored until I actually get to the part where I'm adding that functionality.

@borlaym
Copy link

borlaym commented Sep 17, 2019

The current API didn't let you save it because only the items array is set up as an arbitrary JSON on the backend. To add a new field, some backend work is needed, but I think it's worth doing it. While working on the component in storybook, you can work from a local JSON, and once the structure is final, I think it wouldn't take long for @vtanyi-gmg to implement it.

@vtanyi-gmg
Copy link

@BriceShatzer Yeah, currently only the layout and items fields are arbitrary JSON, not the whole curation object. If that helps, I can add a top-level customHeader field that's also arbitrary JSON, and we can refine it later. (Although after-the-fact data changes a bit messy, because we have to change the data with the old format while keeping the whole thing working, so it helps if the data is in a schema-friendly format.) For images specifically, I think using the already existing models like SimpleImage and MediaAlignment etc. when possible is a good default.

@borlaym
Copy link

borlaym commented Sep 17, 2019

If you can promise it will be fixed later, I'm fine with that, but I also think there wouldn't be any issues if we followed the workflow I outlined above.

@vtanyi-gmg
Copy link

@BriceShatzer So the background, primary, secondary, tertiary fields are all optional, right?
Also, how would you feel about bringing the image fields up one level, to the level of type?

// Current:
{
  "primary": {
    "type": "image",
    "value": { "format": "png", "id": "1234", "altText": "aaaa" }
  }
}
// Proposed:
{
  "primary": { "type": "image", "format": "png", "id": "1234", "altText": "aaaa" }
}

But if the current way is easier to handle on the frontend this is also fine, it's just a minor convenience thing.

@BriceShatzer
Copy link
Author

BriceShatzer commented Sep 19, 2019

@vtanyi-gmg so a couple of things...

  • If we do that there, would we want to do it on background also?

  • Wouldn't doing that make this logic turn real gross?

{
    "if": {"properties": {"type": {"const": "image"}} },
    "then": {"properties": {"value": {"$ref": "#image"}} }
}

Also with the potential of allowing more flexibility for different types of things in these fields in the future, I feel like taking the approach of treating these different "containers" (primary,secondary,tertiary) as generically as possible is the correct approach.

With that in mind, tertiary should probably end up being...

"tertiary": {
    "type": "object",
    "properties": {
        "type":  { "type": "string", "enum": ["links"]  },
    },
    "allOf":[
        {
            "if": { "properties": {"type": {"const":"links"}} },
            "then": {
                "properties": {
                    "value": {
                        "type": "array",
                        "items": { "$ref": "#link" }
                    }
                }
            }
        }
    ]
}

@BriceShatzer
Copy link
Author

Also I didn't include any sort of identifier stuff in the schema like categoryId or storyTypeId which should probably be there.
ಠ_ಠ

@vtanyi-gmg
Copy link

vtanyi-gmg commented Sep 20, 2019

@BriceShatzer

  • Yes, I meant an image in every position would have the type on the same level as its other fields. That's similar to how block nodes work for example. I don't think it's a big issue if it's ugly in this particular schema description format, if it's easily typeable both in Flow and Scala we should be fine.
  • Yeah I agree with tertiary being a container just like the others. Tbh I wasn't planning to restrict primary/secondary/tertiary/background to their specific content types on the backend, instead letting each be a color | image | plaintext | links, at least in the first iteration.
  • Yeah, the ids should be a part of this. I propose to have those in an outer layer though, that way I could store the content part in one field in the database and later changes (e.g. we want to drop tertiary or add a new one) are just a code change, not a database change.

So I'm thinking about a model like this in flow syntax, let me know what you think:

// This is what the API takes/returns:
type CustomHeaderJSON = {
  storyTypeId?: StoryTypeId,
  categoryId?: CategoryId, // Either storyTypeId or categoryId must be defined
  content: CustomHeaderContent
  // The backend also stores the last update time and the user who did it on this level, but probably the frontend won't need that.
};

type CustomHeaderContent = {
  background?: CustomHeaderColor | CustomHeaderImage,
  primary?: CustomHeaderPlainText | CustomHeaderImage,
  secondary?: CustomHeaderPlainText,
  tertiary?: CustomHeaderLinks
};

type CustomHeaderColor = {
  type: 'color',
  value: string
};

type CustomHeaderImage = {
  type: 'image',
  id: string,
  format: ImageFormat,
  altText?: string
};

type CustomHeaderPlainText = {
  type: 'plainText',
  value: string
};

type CustomHeaderLinks = {
  type: 'links',
  value: [{ text: string, url: string }]
};

@BriceShatzer
Copy link
Author

Yep that all looks good.
And yeah, I think having multiple different content types that can be interchangeable into any of the different "containers" is where I figured we would eventually end up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment