Skip to content

Conversation

@benjiwheeler
Copy link
Contributor

Resolves

Proposed Changes

  • alertsList.filter in containers/alerts.jsx and containers/inline-messages.jsx was made into a reducer selector
  • extended comment defining fields of alerts in alerts reducer is now a jsdoc
  • renamed SHOW_STANDARD_ALERT since it shows inline alerts too

Reason for Changes

routine code improvement

@benjiwheeler benjiwheeler added this to the November 2018 milestone Nov 26, 2018
@benjiwheeler benjiwheeler requested a review from rschamp November 26, 2018 01:58
@benjiwheeler benjiwheeler changed the title Alerts revise Revise alerts code to clarify and reorganize alert type handling Nov 26, 2018
Copy link
Contributor

@rschamp rschamp left a comment

Choose a reason for hiding this comment

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

I think the only thing important to change or remove is the SHOW_ALERT comment, since it's still confusing/possibly misleading.

rschamp
rschamp previously approved these changes Nov 29, 2018
@benjiwheeler benjiwheeler merged commit 0191f97 into scratchfoundation:develop Nov 29, 2018
@benjiwheeler benjiwheeler deleted the alerts-revise branch November 29, 2018 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants