This will only fire if isActive is false.

Is there a better way to do this? I can't import store from edit-post, I guess it's because edit-post imports editor store. Memory error when I tried.

I copied this from edit-post selectors.

Not sure if there is a better way to do this but I require this value from edit-post store.

I refactored this to use isFeatureActive now that it is defined here.

The editPost store shouldn't be used in the editor package.

select( preferencesStore ).get( 'core/edit-post', feature ); this is also weird (I know it's not introduced in this PR) because it's fetching a preference of edit-post in editor package. cc @talldan

The reason it shouldn't be used is because it creates a cycle dependency. I'm not sure what you're trying to achieve here but you should probably move your logic to edit-post.

@youknowriad When the welcome dialog opens and you select the Close button, focus just kind of gets lost in the void. The goal was to focus the post title field. I thought it might be easiest to fire the focus from the useEffect already defined for post title focus.

1. Create a new post.
2. Welcome dialog appears.
3. Select Close button.
4. Focus is placed in post title field because the post is empty.

This works fine when the welcome dialog does not appear.

The PostTitle component though is a generic component that don't assume context (edit-post editor, template mode...). So either we find a generic heuristic and put it in the component or instead move the logic elsewhere (edit-post which has the context about everything)

@youknowriad I used a different approach. Hopefully this looks better. Also added a new E2E test so you can see the expected behavior.

select( preferencesStore ).get( 'core/edit-post', feature ); this is also weird (I know it's not introduced in this PR) because it's fetching a preference of edit-post in editor package. cc @talldan

It looks unusual, but I don't think select( preferencesStore ).get( 'core/editor', feature ) is right. The publish sidebar is part of the post editor and this piece of code says to get whether the post editor's sidebar is enabled. The core/edit-post part refers more to the application rather than the package. If a package name like 'core/editor' were used, this becomes seperate from the post editor's other preferences and it becomes hard to know what the preference is for, and whether is should apply to other editors.

Perhaps the issue is that the selector shouldn't be in the editor package, it could be deprecated here and moved to the post editor.

I read a post where using "_props" is okay in this context but not sure.

I think since this component don't use any props, the argument is not used anyway, so you can name it whatever. In general I use _ as a name for situations like that but _props is fine too I guess.

Changed.

I make sure post is clean here to not requiring access to it before this function runs.

Should this be more something like isWelcomeGuideVisible?

Changed.

If I'm not wrong the PostTitle component is not visible at all in the template mode (it's not rendered), meaning we should either change the element we focus on, or avoid focusing anything entirely.

This will only happen for normal welcome guide now.

Should this be just "focus"? it's already the "PostTitle" component so maybe focusTitle is a bit redundant

Changed.

I think this check should move outside of "post-title" component. Because it's not clear why "calling" ref.focus should abort if the post is new. So we can move this logic instead to https://github.com/WordPress/gutenberg/pull/40195/files#diff-175b6822ebac715bc984ae982f1d9230b5d1e441c98138d8a1fc7ccf275c19bcR194 (or remove it entirely)

@youknowriad I thought it would be better to keep it there since it was already selected. Anyway, moved it to edit-post.

A side effect of adding the value of isCleanNewPost as a dependency here is that the effect will run if isCleanNewPost changes even if isWelcomeGuideVisible didn't change. Meaning if the post was not clear but becomes clean (could happen programmatically for instance) while the welcome guide is open, the title will be focused and the welcome guide closes (focus moved outside).

The probably of the above happening is small of course but I think there's a better way to implement this logic. (Calling the selector inside the effect).

So you'd select the selector by doing :

const { isCleanNewPost } = useSelect( editorStore );

and then in the effect, you'd replace ! isCleanNewPost with ! isCleanNewPost() instead.

Note that this technique of calling selectors outside of useSelect should only be used if the returned value is not used in the rendering of the React tree but in an effect or event handler or any random callback.

@youknowriad I get this error.

React Hook "useSelect" cannot be called inside a callback. React Hooks must be called in a React function component or a custom React Hook function.

What should I do?

1. Ignore it with a comment.
2. Add a comment explaining this effect will only fire if post is clean as defined in editor. Then not use isCleanNewPost in edit-post and leave that functionality in editor.
3. Other ideas?

Actually, I guess it will not work. I generated a local build and this was logged in console.

Uncaught Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.

The useSelect call shouldn't be inside the callback but it should just replace the existing one line 116

Ah, okay. Fixed. Just the 1 call to useSelect( store_name ); retrieves once, yes? That way, once it is retrieved, it will not trigger the effect again?