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

Make snack bar messages persistent by default #1284

Merged
merged 1 commit into from
Jun 18, 2018

Conversation

brusherru
Copy link
Contributor

It closes #1234

@brusherru brusherru self-assigned this Jun 13, 2018
@brusherru brusherru requested a review from a team June 13, 2018 17:04
@brusherru brusherru force-pushed the tweak-1234-persistent-messages-by-default branch from 20c31bc to 2ca94ce Compare June 13, 2018 17:06
type: ActionType.MESSAGE_ADD,
payload: messageData,
meta: {
id,
type,
persistent,
persistent: R.propOr(true, 'persistent', messageData),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could it default to true only if type is MESSAGE_TYPE.ERROR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it and even did it at first. But when I began to place persistent flags in message objects, I realized that there is confusion. So I decided to do it persistent by default for all types of messages.

@brusherru brusherru force-pushed the tweak-1234-persistent-messages-by-default branch 2 times, most recently from c486678 to 650723c Compare June 14, 2018 09:59
@brusherru
Copy link
Contributor Author

@nkrkv @evgenykochetkov just fixed it. Check it out, please.

persistent = false,
id = null
) => ({
const getDefaultPersistencyByType = type => {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just const getDefaultPersistencyByType = R.equals(MESSAGE_TYPE.ERROR)?

case MESSAGE_TYPE.CONFIRMATION:
case MESSAGE_TYPE.NOTIFICATION:
default:
return false;
Copy link
Member

Choose a reason for hiding this comment

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

The whole function can be simplified to type === MESSAGE_TYPE.ERROR.

@brusherru brusherru force-pushed the tweak-1234-persistent-messages-by-default branch from 650723c to bf2d9ff Compare June 14, 2018 14:27
@brusherru
Copy link
Contributor Author

My bad 🙈
@nkrkv Check it out, please.

Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

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

👍

@brusherru brusherru merged commit f464ed5 into master Jun 18, 2018
@brusherru brusherru deleted the tweak-1234-persistent-messages-by-default branch June 18, 2018 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make error snack bar messages persistent by default
3 participants