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

Add missing title dialog #4251

Merged
merged 15 commits into from
Sep 8, 2020
Merged

Add missing title dialog #4251

merged 15 commits into from
Sep 8, 2020

Conversation

barklund
Copy link
Contributor

@barklund barklund commented Aug 27, 2020

Summary

This adds a "missing title" dialog that appears when trying to publish without a title.

This is what it looks like:
Screen Shot 2020-08-31 at 22 45 23

And this is how the flow works:
title_adding

Relevant Technical Choices

  • Added a necessary context to the header to be able to focus the title input from the publish button.

To-do

User-facing changes

  • If you try to publish without a title, you get a dialog.

Testing Instructions

  1. Create a story without a title
  2. Press the publish button and observe a dialog warning about missing title
  3. Press "Add a title" and observe that after the dialog closes, the story will still be in draft state and the keyboard focus will move to the title input

Fixes #4184

@barklund barklund requested review from miina and merapi August 27, 2020 01:45
@barklund barklund self-assigned this Aug 27, 2020
@google-cla google-cla bot added the cla: yes label Aug 27, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Aug 27, 2020

Size Change: +750 B (0%)

Total Size: 1.24 MB

Filename Size Change
assets/js/edit-story.js 497 kB +490 B (0%)
assets/js/stories-dashboard.js 587 kB +260 B (0%)
ℹ️ View Unchanged
Filename Size Change
assets/css/edit-story.css 296 B 0 B
assets/css/stories-dashboard.css 328 B 0 B
assets/css/web-stories-embed-block.css 515 B 0 B
assets/js/chunk-web-stories-template-0-********************.js 10.2 kB 0 B
assets/js/chunk-web-stories-template-1-********************.js 10.3 kB 0 B
assets/js/chunk-web-stories-template-2-********************.js 10 kB 0 B
assets/js/chunk-web-stories-template-3-********************.js 10.5 kB 0 B
assets/js/chunk-web-stories-template-4-********************.js 10.6 kB 0 B
assets/js/chunk-web-stories-template-5-********************.js 6.86 kB 0 B
assets/js/chunk-web-stories-template-6-********************.js 9.91 kB 0 B
assets/js/chunk-web-stories-template-7-********************.js 9.78 kB 0 B
assets/js/web-stories-activation-notice.js 63.6 kB 0 B
assets/js/web-stories-embed-block.js 16.9 kB 0 B

compressed-size-action

Copy link
Collaborator

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to add this to the storybook

@samitron7
Copy link

@o-fernandez Can you help with the messaging here. I'm not sure if we can use 'research shows'.

@o-fernandez
Copy link
Contributor

@o-fernandez Can you help with the messaging here. I'm not sure if we can use 'research shows'.

Right, I'd also eliminate the exclamation marks so it doesn't seem like shouting.

How about this?

Missing Title

We recommend adding a title to the story prior to publishing (learn more)

Button: Add a title || Button: Publish without title

Also, @barklund, is it possible to just bring up an input field into the dialog/pop-up where the user could enter the title directly there? If so, that would be more streamlined and we could have.

Missing Title

We recommend adding a title to the story prior to publishing (learn more).

Title: [EMPTY INPUT FIELD]

button: Cancel || button: Publish

In both of these, learn more would link to the guidelines as it currently does.

@barklund
Copy link
Contributor Author

@o-fernandez:

How about this?

Missing Title
We recommend adding a title to the story prior to publishing (learn more)
Button: Add a title || Button: Publish without title

Works for me!!!!!

Also, @barklund, is it possible to just bring up an input field into the dialog/pop-up where the user could enter the title directly there? If so, that would be more streamlined and we could have.

Missing Title
We recommend adding a title to the story prior to publishing (learn more).
Title: [EMPTY INPUT FIELD]
button: Cancel || button: Publish

I would like a design for that, but it could be done. Note that the overlay has white background, but our input fields have dark background - thus the need for a design.

In both of these, learn more would link to the guidelines as it currently does.

That link to guidelines doesn't work outside of Google. I can't see the link - I just get a "You need access" page:

Screen Shot 2020-08-31 at 13 43 27

This is the link I'm using:
https://drive.google.com/drive/folders/1dqPtjNTYN7OxTngWAYhy8zn8X3EuRTGJ (try opening in incognito)

@o-fernandez
Copy link
Contributor

o-fernandez commented Aug 31, 2020

@barklund I'd use this link

Ok if, for now, we don't have the input field in the dialog box... don't want this to be blocked on design. But, it'd be nice to figure it out if possible.

@barklund
Copy link
Contributor Author

barklund commented Sep 1, 2020

@samitron7, please check the updated design above in the PR description at the very top. Does it look acceptable?

@codecov
Copy link

codecov bot commented Sep 4, 2020

Codecov Report

Merging #4251 into main will increase coverage by 0.14%.
The diff coverage is 95.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4251      +/-   ##
==========================================
+ Coverage   83.37%   83.52%   +0.14%     
==========================================
  Files         845      853       +8     
  Lines       14937    14964      +27     
==========================================
+ Hits        12454    12498      +44     
+ Misses       2483     2466      -17     
Flag Coverage Δ
#karmatests 71.87% <63.15%> (+0.20%) ⬆️
#unittests 66.43% <92.10%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...rc/edit-story/components/header/buttons/preview.js 85.29% <85.29%> (ø)
assets/src/edit-story/components/dialog/index.js 100.00% <100.00%> (ø)
.../src/edit-story/components/header/buttons/index.js 100.00% <100.00%> (ø)
...rc/edit-story/components/header/buttons/publish.js 100.00% <100.00%> (ø)
...t-story/components/header/buttons/switchToDraft.js 100.00% <100.00%> (ø)
...src/edit-story/components/header/buttons/update.js 100.00% <100.00%> (ø)
assets/src/edit-story/components/header/context.js 100.00% <100.00%> (ø)
...s/src/edit-story/components/header/headerLayout.js 100.00% <100.00%> (ø)
...ssets/src/edit-story/components/header/provider.js 100.00% <100.00%> (ø)
assets/src/edit-story/components/header/title.js 94.73% <100.00%> (+16.95%) ⬆️
... and 15 more

@barklund barklund marked this pull request as ready for review September 4, 2020 05:26
@barklund barklund requested review from miina and swissspidy September 4, 2020 05:26
Copy link
Contributor

@miina miina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome refactor + tests! 🎉

One finding from testing: it doesn't seem like it's actually possible to publish without a title, there's a console error, with REST response:
Screenshot 2020-09-04 at 10 53 48

The story stays unpublished.
Note: tested with a new story + adding just one media.

@barklund
Copy link
Contributor Author

barklund commented Sep 4, 2020

One finding from testing: it doesn't seem like it's actually possible to publish without a title, there's a console error
The story stays unpublished.

I don't see any errors and the story publishes just fine for me. Maybe it has to do with permalinks settings, as my setup is set to default (id only) and the slug then for a new story without title defaults to the ID of the story. It sounds like you're trying to set an empty string as slug or similar? Also, what should happen in this case?

@barklund
Copy link
Contributor Author

barklund commented Sep 4, 2020

One finding from testing: it doesn't seem like it's actually possible to publish without a title, there's a console error
The story stays unpublished.

I don't see any errors and the story publishes just fine for me. Maybe it has to do with permalinks settings, as my setup is set to default (id only) and the slug then for a new story without title defaults to the ID of the story. It sounds like you're trying to set an empty string as slug or similar? Also, what should happen in this case?

I'm not seeing any issues publishing a story without a title (and slug) - even with permalink set to something involving the slug.

@miina
Copy link
Contributor

miina commented Sep 7, 2020

I'm not seeing any issues publishing a story without a title (and slug) - even with permalink set to something involving the slug.

For some reason, I'm also not seeing this anymore 🤷

@barklund barklund merged commit ef538af into main Sep 8, 2020
@barklund barklund deleted the add/4184-missing-title-dialog branch September 8, 2020 21:38
@swissspidy swissspidy added the Type: Enhancement New feature or improvement of an existing feature label Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Canvas Group: Help Center For first-time user experience (FTUE). Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prompt user to add a story title when publishing, if not set already
6 participants