Implement the required metadata fields check in the editor

[ingeniumed]

Description

• Implement the required metadata fields check in the editor, based on the status of the post. Currently, this is just on the frontend as none of the backend checks I've tried to implement have successfully worked. They seem to throw an error, but still allow status movements. I'm trying to figure out a good way to block this but for now that's not there.
• Show only the previous statuses in the status dropdown, with the intention being to remove this whole thing eventually. This stops a weird edge case where you could advance the status even if you didn't have permission to.
• Add a new icon for the workflow plugin thanks to @jarekmorawski.
• Update the tooltip for the disabled publish/advance button to be Requirements for advancing have not been met. Ideally, there'd be more done but the purposes of this PR this shows the whole thing working end to end.
• Add a red asterisk against every required field as well.

Problems in this PR that still have to be fixed in follow up PRs

• The red asterisk sometimes doesn't show up immediately, haven't been able to speed this up more without causing stability problems.
• Backend checks for stopping any workarounds for required editorial metadata fields is missing right now. I haven't been able to get those to consistently work, as the post meta value that's being inserted isn't given back. It's what's saved which at the moment is nothing. So it stops an update to the status that includes post meta values. Plus, sometimes you can click save draft to save the new status despite this failure. All in all, not stable enough to include it imo.
• UI inconsistencies still exist between editorial metadata fields, the extended post status and the preview link since they aren't aligned properly, etc.

[jarekmorawski]

It's looking great! Can we also double-check the spacings to make sure the gaps between our fields are the same as between the core status fields in the upper part of the summary panel? I also feel that the space before and after the line separator is too great and people would appreciate a bit denser UI. Not sure what it is right now, but it's 16 px in the design.

[ingeniumed]

I tried to do the best I could, but it's still slightly off. We will keep working on that in follow up PRs so it looks more native.

A big change that I did make in this PR now is:

I switched the extended post status to be a button, that triggers a popover similar to the existing status option and how we show the editorial metadata. I haven't shown the description in here for now.

[alecgeatches]

@ingeniumed @jarekmorawski The first change I'm noticing in this PR is the icon. It looks too dark and off-center:

Notice the icon is a pure black here:

Also, when selected, the icon remains dark and the off-centerness is more obvious:

Compare this to regular menu icons, which have a few changes related to state:

1. The icon is a gray (rgba(240, 246, 252, 0.6)) color when inactive.
2. In a hover state, the icon turns blue (#72aee6) to match the surrounding text.
3. When a menu section is clicked and the page loads, the icon is white.

Our icon currently stands out due to the positioning and color. We may need something more complex than a single SVG for this icon to better match the WordPress UI. I'd be happy to circle back here and figure out how we might be able to do that.

[ingeniumed]

All valid points - I should have waited a bit more but I was excited to add in an icon for the plugin 😟

How about I remove this icon for now? Let's add back in once we have those three sorted rather than leaving it in, in a state that's not ideal.

[alecgeatches]

@ingeniumed Not sure if this slipped through in another PR, but I'm seeing that EM field changes in configuration don't persist on-page:

The field is properly saved after a page refresh, but without a full page refresh EM fields on a status are forgotten. Users seem to persist correctly though.

[alecgeatches]

Would it be possible to have two messages here? One for when a status requires action (e.g. check a box), and one for when another user needs to make the change. I found myself looking through EM fields for a red asterisk whenever a status had a required user. This could be a frustrating experience for a user if it's unclear what is blocking movement when it's out of their control.

@jarekmorawski any thoughts or suggested copy here?

[ingeniumed]

This was part of what's not included in this PR yet - the UI inconsistencies, the red asterisk taking some time to show and missing backend checks for EM fields being required.

Def agreed on this though that it needs to be improved further so it's not frustrating or unknown as to why it's blocked.

[alecgeatches]

Minor: this feels a bit awkward to search through all of $editorial_metadatas for each term. I also found this a bit hard to follow and needed a debugger to ensure I understood it.

One way to handle something like this would be precomputing the mapping of metadata IDs to terms in something like $metadata_id_to_term computed once, and then just check the mapping in constant-time when needed instead of a second array_filter(). This isn't really an optimization issue as it wouldn't be a problem until there are thousands (or more) of both metadata terms and custom status associations, but I do think it could help the readability.

[ingeniumed]

Same thing as below really - this can be optimized a lot more once the refactor + bottom optimization is in. This way causes the least amount of conflicts or problems with the current code.

[alecgeatches]

Any thoughts about making this default get_custom_statuses() behavior? One thing that trips me up is we set meta[ self::METADATA_REQ_EDITORIAL_IDS_KEY ] by default but meta[ self::METADATA_REQ_EDITORIALS_KEY ] is computed only when this function is called. I don't like that we need to remember which of these are provided by the base function and which are computed by other functions that need to be called later.

I think these keys ideally represent all available data on custom statuses, and have helper functions to provide computed results (e.g. get_required_editorial_metadata($term_id)) and not store the result in meta, since it's derived data. It's just handling both actual metadata and computed results in the same place that's confusing, I think.

[ingeniumed]

So, that is something I want to do but we need to finish the custom status migration before we can do that due to the init conflicts that arise. The priorities hacks worked sometimes, but they also caused issues sometimes. The problem is that EM's taxonomy gets used in CS and vice versa.

I did some experimentation with #59 and it worked a lot better. So I've been waiting to get that in and then I'll tweak this.

[alecgeatches]

Overall, this is great! I love the new EM field UI in the editor and it's working almost exactly like I'd hoped. Fantastic job!

We had a conversation about the outstanding issues. From my understanding, we can merge this PR after:

• Fixing the EM field configuration not staying on the page when saved.
• Removing the current icon as it has some issues in the sidebar.

We'll return in future work to finish the remaining UI polish issues noted:

1. Having a separate message for missing EM fields vs missing user.
2. Refactoring how we handle computed EM metadata, which can be done after the major refactor in Refactor the remaining modules #59 is merged.
3. Minor optimizations in how we gather EM metadata.

Thank you!

[ingeniumed]

Yip, that's correct.

What I've done:

• Removed the icon
• Updated the metadata-id handler to add the computed EM Metadatas in. My intention is that, once Refactor the remaining modules #59 is in this will be the only way that it will be populated. That way, no one needs to worry about it as it'll be already done. That will delete the special private method as well.

I've filed the following as follow ups:

#60 and #61