-
Notifications
You must be signed in to change notification settings - Fork 2
Update the design for the required fields on a custom status #55
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
Conversation
| const [ isRequesting, setIsRequesting ] = useState( false ); | ||
| const [ isRestrictedSectionVisible, setIsRestrictedSectionVisible ] = useState( | ||
| requiredUsers.length > 0 || requiredMetadatas.length > 0 | ||
| const [ areRestrictedUsersSet, setAreRestrictedUsersSet ] = useState( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not the biggest fan of setting text here to control the radio control, so happy to change this. This seemed like the best way imo for now.
|
Note: I've dropped roles from the verbiage but the screenshots still reflect the old verbiage. Just noting that, as we will add roles support in the future. |
|
It's looking good! The only part I'm unsure of is the required metafields label. It isn't clear to me what the field does. Would it make sense to change it to something like below?
Note that it's the inversion of the logic suggested in the new design where users would set visibility requirements on the succeeding status (e.g., 'Legal review' would only be visible if the 'Needs legal review' metafield is filled), instead of the current one (e.g., only move forward if X). I can't say if any of the options is better in the long term, but it does impact the UX and may cause potential complications later on. cc @smithjw1 |
|
Something I had gone back and forth on, was keeping that help text as it makes it clear. I ended up removing that help text in favour of the label being descriptive enough. I'll add it back in, and shift the required metadata fields above the users. Given the user input is hidden, it'll prevent the metadata fields from being pushed down. We would want to keep the users and metadata fields requirement being applied on the current status rather than the next one, as that keeps the whole thing simple. |


Description
This PR is meant to update the design for configuring the required fields on a status, using the mockups proposed by @jarekmorawski. Check out #44 for an example of this.
The changes I've made:
Note: The rules engine is not in place for editorial metadata just yet. Once it is, that section would change a bit more so keep that in mind when reviewing this.
Steps to Test