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

feat(notices): add multiple message support to give-message #1936

Closed
ravinderk opened this issue Aug 1, 2017 · 7 comments

Comments

@ravinderk
Copy link
Collaborator

commented Aug 1, 2017

Issue Overview

Currently, we can only show one message by set give-message URL param. It will be useful if we add array support to it.

Current: give-message=message1
Propposed: give-message[]=message1&give-message[]=message2&give-message[]=message3

For ref: We want to show multiple messages in this PR but unable to do that #1914

Todos

  • Tests
  • Documentation

@ravinderk ravinderk added this to the 1.8.12 milestone Aug 1, 2017

@ravinderk ravinderk assigned ravinderk and unassigned mehul0810 Aug 1, 2017

@ravinderk ravinderk modified the milestones: 1.8.13, 1.8.12 Aug 1, 2017

@DevinWalker DevinWalker assigned raftaar1191 and unassigned ravinderk Aug 14, 2017

@DevinWalker DevinWalker modified the milestones: 1.8.14, 1.8.13 Aug 23, 2017

@raftaar1191

This comment has been minimized.

Copy link
Member

commented Sep 26, 2017

Working on this

@raftaar1191

This comment has been minimized.

Copy link
Member

commented Sep 26, 2017

Chaing the give-message to array will break all the other give addon as they using this variable to process their logic in string format

Example:

@ravinderk

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 26, 2017

@DevinWalker This issue will affect some add-ons need to create a strategy. I am bumping this up to 1.8.15 because it is not urgent.

@ravinderk ravinderk modified the milestones: 1.8.14, 1.8.15 Sep 26, 2017

@ravinderk ravinderk self-assigned this Sep 26, 2017

raftaar1191 added a commit to raftaar1191/Give that referenced this issue Sep 26, 2017
raftaar1191 added a commit to raftaar1191/Give that referenced this issue Sep 26, 2017
@raftaar1191

This comment has been minimized.

Copy link
Member

commented Sep 26, 2017

Commit Not related to the above issues. It's for #1959:

DevinWalker added a commit that referenced this issue Sep 27, 2017
Merge branch 'release/1.8.14' into release/1.8.15
* release/1.8.14:
  Added additional inline text for context
  Fix # 2134
  Fix #2133
  Set run update button right margin to 10px
  Fix run update notice bar buggy animation
  Refix JS error #1959
  Add margin to update button #1959
  Remove Notices on update button click #1959
  Add support to attachment id field
  Add hidden field support
  Remove unused fx
  Improve code formatting and remove duplicate array data
  Fixes #2130 Adding descriptions to Importer fields
  Fixes #2128
  Fixes #2126
  Add script 1936
  Add html and fix formatting #1936
  Add localize script text #1936

@DevinWalker DevinWalker modified the milestones: 1.8.15, 1.8.16 Oct 19, 2017

@ravinderk ravinderk modified the milestones: 1.8.16, 2.1 Oct 23, 2017

@DevinWalker DevinWalker removed this from the 2.1 milestone Mar 6, 2018

@DevinWalker DevinWalker removed the enhancement label Mar 9, 2018

@DevinWalker DevinWalker changed the title Add multiple message support to give-message feat(notices): add multiple message support to give-message Mar 20, 2018

@raftaar1191

This comment has been minimized.

Copy link
Member

commented May 10, 2018

Slack Call Summary

Participants: @raftaar1191, @ravinderk
Topic: How we can tackle this issues
Result:
As suggested by @ravinderk over the call that we should introduce Give Notice because it is difficult to add backward compatibility to addon which is using Give Message.
I also agreed with @ravinderk on this, As this is very much complex if we try to add multi support with backward compatibility for the message as this will make a lot of change in Add-on.

Summary

Replace message with Notices

@DevinWalker ^^

@raftaar1191

This comment has been minimized.

Copy link
Member

commented May 22, 2018

@ravinderk

I guess we should change the notification key name to give-messages

Example: give-messages[]=message1&give-messages[]=message2&give-messages[]=message3

@ravinderk

This comment has been minimized.

Copy link
Collaborator Author

commented May 22, 2018

@raftaar1191 we can do that.

@DevinWalker DevinWalker modified the milestones: Sprint: 2018/05/08 - 2018/05/22, Sprint: 2018/05/08 - 2018/06/05 May 23, 2018

ravinderk added a commit that referenced this issue May 28, 2018
Merge pull request #3242 from raftaar1191/issue-1936
feat(notices): add multiple message support to give-message #1936
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.