Skip to content
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

Explore using a modal to replace the publish panel #16715

Draft
wants to merge 8 commits into
base: master
from

Conversation

@brentswisher
Copy link
Contributor

commented Jul 23, 2019

This is a rough attempt at adding a modal as described in #15847. This is a first step toward enhancing the publishing flow, based off of the designs by @sarahmonster and others tracked in #7602. It will need some additional work, but I thought it would be useful to get some feedback on it at this stage before continuing. The conversation about these changes is a bit fragmented so I am hoping that having some code to look at will help prioritize next steps.

@sarahmonster - Thank you for the great design work, you will notice some of the wording different from the mockups. In working through the code there were some variations between the existing panel and the mockups, usually around wording for scheduled posts. Where it made sense to me I kept the same messages if they were similar enough, but I'm happy to change as needed.

Open to any and all criticisms and suggestions. I used the existing postPublishPanel as a rough framework, and I need to review if some of the logic makes sense as a modal. I'll continue to update this as I have availability and depending on if it's looking like a direction worth exploring.

Additional plans for now:

  • Review componentDidUpdate in the postPublishModal component, I believe this is unnecessary with it being a modal but wanted to test further before removing.
  • Writing unit tests
  • Review for accessibility using https://www.w3.org/TR/wai-aria-practices-1.1/examples/dialog-modal/dialog.html as a guideline for a multi-modal flow (I know there was some concern about changing the title of a modal mentioned in one of the issues)
  • Clean up design of the post-publish page
  • Explore adding tags and post format suggestions back, but in the post-publish view

brentswisher added some commits Jun 28, 2019

@brentswisher brentswisher force-pushed the try/publish-via-modal branch from ccab890 to d6abe21 Aug 23, 2019

brentswisher added some commits Aug 23, 2019

Remove toLowerCase from postLabel because while this fixes the capita…
…lization in english, it would be incorrect in other languages. Will have to explore if this exists somewhere already that would work for all languages.

@brentswisher brentswisher self-assigned this Aug 24, 2019

@brentswisher

This comment has been minimized.

Copy link
Contributor Author

commented Aug 24, 2019

I have made some updates to bring this into line with an updated prototype in #7602 (comment). I also made a few intentional changes:

  • Left the "Always use these pre-publish checks" checkbox above the cancel and publish button. I believe for accessibility it is generally better to have the submission buttons at the end of a form
  • Set a standard width for the modal per @mapk's feedback in the issue.
  • Left a "Publishing..." modal with the spinner animation - This is closer to what is there now, and I found the modal blipping in and out very disorienting. I know there are some accessibility concerns with this changing the title of the modal mentions somewhere. My hope is to match this up to the techniques in the delivery address demo here to remedy any issues. But feedback from someone on the accessibility team would be great.

Leaving this as a Draft PR as there is quite a bit of back and forth in #7602 about whether a modal is a step forward or not. Hoping that this work will move the conversation forward by being able to experience it, but understand this is far from a settled direction for the publishing flow. (I will also work on getting a version up with this prototype in a full screen modal early next week @karmatosed)

@brentswisher brentswisher referenced this pull request Aug 24, 2019
2 of 8 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.