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

NUX: bootstrap a NUX module #4173

Closed
wants to merge 1 commit into from
Closed

NUX: bootstrap a NUX module #4173

wants to merge 1 commit into from

Conversation

youknowriad
Copy link
Contributor

refs #3670 Still WIP but worth a review

This PR bootstraps the work on the NUX.

Functional Side:

  • This doesn't do much aside adding an example "dot" tip for the inserter. It's just an example for now. The main purpose of this PR is to setup the technical aspects
  • a DotTip show up only once. Once you close it or reload the page, it will never show again.

Technical Side:

  • I added a nux module allowing any WP module to add "tips".
  • This module can be used by using a Provider and then inserting several DotTip components in the different places we want to show tips. (Other type of tips will be added later)
  • This module uses the data module to store a local state
  • This local state is persisted to local storage for now. This is not great, it's better if we could persist this server side but the "user preferences" API is not good enough to store non string values. It could be changed later.
  • I've moved the persist enhancer to the data module as helpers and updated it to be able to use it several times.

Testing instructions

You can reset the state of the already shown dots by clearing the local storage.

@youknowriad youknowriad added the [Feature] NUX Anything that impacts the new user experience label Dec 26, 2017
@youknowriad youknowriad self-assigned this Dec 26, 2017
* @return {Boolean} Whether the tip has been shown or not
*/
export function tipHasBeenShown( state, tipId ) {
return state.nux.tips[ tipId ];
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance this would return undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, I think it's possible which still produces a "falsy" value but might better to force false

@karmatosed
Copy link
Member

karmatosed commented Jan 3, 2018

Firstly, thanks @youknowriad for a great start on this! I really do like the pulsing dot. I am not so sure we need to add in a speech arrow to the block though as that adds in confusion, could we have more like the mockup in future iterations? Similarly, could it be to the side and wider? These are all cosmetic suggestions.

Here is a visual of what we have right now:

2018-01-03 at 14 32

Excited to see how this shapes up more!

Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

Added a few visual changes I'd like to see as a comment.

@aduth
Copy link
Member

aduth commented Jan 3, 2018

Does this need to be a separate module, or merely a small set of components files? Is it required to be this way because it manages its own state (and primarily to take advantage of existing persistence helpers)? I guess this is in line with the thinking that an individual module has 0 or 1 stores? Would we ever consider accommodating the need for component stores without needing to extract into separate modules?

@youknowriad
Copy link
Contributor Author

Is it required to be this way because it manages its own state (and primarily to take advantage of existing persistence helpers)?

Yes, that's the idea. The persistence though is just a convenience though. I'd prefer if we had a server-side API for user preferences here.

Would we ever consider accommodating the need for component stores without needing to extract into separate modules?

The advantage of separating modules is the possibility to use it in other pages than the "editor" page. This could be useful in any WP page.

Does this need to be a separate module, or merely a small set of components files?

My understand of the components module is a set of stateless generic UI modules.

@youknowriad
Copy link
Contributor Author

Closing this for now, I'll revisit on clean PR when the time comes

@youknowriad youknowriad deleted the try/nux branch April 20, 2018 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] NUX Anything that impacts the new user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants