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

Checklists: State and structure #26015

Closed
sirreal opened this issue Jul 12, 2018 · 5 comments
Closed

Checklists: State and structure #26015

sirreal opened this issue Jul 12, 2018 · 5 comments

Comments

@sirreal
Copy link
Member

sirreal commented Jul 12, 2018

This issue is to share some discussion that @ockham and I have been having while looking at #25956

I'll summarize conversations up until this point and provide a bit of a roadmap. We're taking on some checklist work.

  • tasks is a mix of structure and state.
  • We'd like state to be minimal and live in Redux.
  • We'd like structure to be declarative React.

Here's tasks now (or soon):

export const tasks = [
{
id: 'jetpack_brute_force',
completedTitle: translate(
"We've automatically protected you from brute force login attacks."
),
completed: true,
},
{
id: 'jetpack_spam_filtering',
completedTitle: translate( "We've automatically turned on spam filtering." ),
completed: true,
},

It's connected and merged with server state in connect:

const mapStateToProps = ( state, { siteId } ) => {
const tasksFromServer = get( getSiteChecklist( state, siteId ), [ 'tasks' ] );
const tasks = tasksFromServer
? mergeObjectIntoArrayById( onboardingTasks, tasksFromServer )
: null;

Note that this is undesirable because:

  • It does a lot of work in connect
  • A new object is created every time connect runs, meaning the component will likely rerender.

Then a checklist is rendered like this:

{ this.getUncompletedTasks().map( this.renderTask ) }


What if a checklist looked like this?

No more tasks state and structure.

Each checklist looks more like this. Note that all of these structural things live here declaratively (task ids, urls, translated strings…)

<SomeChecklistWrapper>

<ChecklistItem
  taskId="jetpack_brute_force"
  completedTitle={ translate( "We've automatically protected you from brute force login attacks." ) }
  status="complete"
/>
<ChecklistItem
  taskId="jetpack_spam_filtering"
  completedTitle={ translate( "We've automatically turned on spam filtering." ) }
  status="complete"
/>
<ChecklistItem
  taskId="jetpack_monitor"
  title={ translate( 'Jetpack Monitor' ) }
  description={ translate(
    "Monitor your site's uptime and alert you the moment downtime is detected with instant notifications."
  ) }
  completedTitle={ translate( 'You turned on Jetpack Monitor.' ) }
  completedButtonText={ translate( 'Change' ) }
  duration={ translate( '3 min' ) }
  tour={ 'jetpackMonitoring' }
  url={ '/settings/security/$siteSlug' }
/>

</SomeChecklistWrapper>

And our ChecklistItem blocks looked something like this?

// Rendered mostly the same…
export default connect( ( state, props ) => ( {
  status: props.status || getTaskStatus( state, props.siteId, props.taskId ),
} ) )( ChecklistItem )

Now, we'd imaging that Redux state would be very minimal, including only the status of a particular task (pending, in progress, complete, dismissed?).

@ockham
Copy link
Contributor

ockham commented Jul 12, 2018

Love it ❤️ It makes so much more sense to put stuff like title, description, or tour directly into React components.

@sirreal
Copy link
Member Author

sirreal commented Jul 12, 2018

Noting that completed tasks move up, which changes the declared ordering. We can handle that with flexbox magic.

css idea
.SomeChecklistWrapper {
  display: flex;
  flex-direction: column;
}

.ChecklistItem {
  order: 100;
}

.ChecklistItem.is-complete {
  order: 0;
}

@sirreal
Copy link
Member Author

sirreal commented Jul 12, 2018

@markryall @taggon @timmyc Wanted to involve you in this conversation as you've been involved in checklists.

@sirreal
Copy link
Member Author

sirreal commented Jul 12, 2018

See implementation ideas in #26030 #26216

@sirreal
Copy link
Member Author

sirreal commented Aug 16, 2018

Implimented in #26618

@sirreal sirreal closed this as completed Aug 16, 2018
@sirreal sirreal self-assigned this Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants