-
Notifications
You must be signed in to change notification settings - Fork 13
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
Oops we forgot about this one! Delivery options on the editing screen #4768
Comments
Can we just remove the modal / "more" button and just keep the "purple bar" / quick select ? Noting the current: Like the form purpose I didn't add all the API stuff to this dialog. If an API key exists I do hide the delivery option. |
If we keep the bar, we could remove the options to select anything (just show what is selected) and direct people to the settings tab with the button rather than open a modal. |
I think we could probably remove the modal without losing anything? The specific settings (classification and delivery method) are now highlighted on the publishing checklist, which I don't think they were when we added this originally. I also wonder how many folks chose branding from the purple bar versus in settings, it might be a bit redundant to have it in both spots especially given it's really a one-time choice and shows up on "Test" page with the preview as well? |
I agree |
PR to remove This feature has always been quite hidden / tucked away under the This will help on the App maintenance / testing end. All of the functionality + more is available under settings. 541 lines of code remove in that PR. |
Maybe 👍 before I merge that PR. |
The PR LGTM! I agree, we've talked about the redundancy of that modal for a while now. Before we merge, flagging to @srtalbot for visibility |
When we added the administrative purpose, which has no default and forces the user to go to the settings page, the original problem this bar was created to solve resolved itself. Which was to surface those settings before the user could publish. |
We have three options here:
@anikbrazeau @samsadasivan @timarney @Abi-Nada
The text was updated successfully, but these errors were encountered: