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

fix: closing notification does not mark notices read #408

Merged
merged 1 commit into from
Oct 4, 2019

Conversation

JalilArfaoui
Copy link
Member

There is nothing pushed yet for this PR, but I can already expose what I'm doing and the reasoning behind.

Actually, I had to go paper and pen on this one to have a clear view of what is happening.

What I found out is that the issue we're having is to me a consequence of having 2 synchronization mechanisms between content and background :

  • UNFOLD and FEEDBACK actions are sent directly to the background, and the background is responsible for applying domain rules and updating the state.
  • MARK_READ and FEEDBACK is handled in the content, that then send a generic NOTICES_UPDATED to background, but currently this actions is only used by the background to update the browser action.

image

So, I figured we had to simplify this to a common mechanism to avoid further trouble.

I initially tried to send everything through NOTICES_UPDATED in both ways, and stop transmitting UNFOLD, CLOSED and FEEDBACK :

image

But I quickly saw a difficulty we were going to have. When receiving NOTICES_UPDATED actions in background, we would have a hard time figuring out what changed (read status ? feedback ? ...) and what to update in state.

So, I changed my plan to do the opposite : Get rid totally of NOTICES_UPDATED and send simple MARK_READ and FEEDBACK actions from content to background. And then we can update the browser icon saga to be triggered whenever we have MARK_READ, FEEDBACK or NOTICES_FOUND actions dispatched.

image

I think I'm almost done with this second plan, and I push it ASAP.

But if you have any comment to make, I would be happy to hear read

@lutangar
Copy link
Member

lutangar commented Oct 3, 2019

I like this one:
image
Plus, we can leverage code reuse between background and content. (reducer? action? selectors (coming soon, need to discuss that)

@JalilArfaoui JalilArfaoui force-pushed the fix/closing_does_not_mark_as_read branch from 048f35f to a5a37d6 Compare October 3, 2019 14:40
@JalilArfaoui
Copy link
Member Author

I pushed a first working version.

I still want to refactor some things I'm unhappy with (see my FIXME comment).

@lutangar
Copy link
Member

lutangar commented Oct 4, 2019

@JalilArfaoui still a WIP?

@JalilArfaoui
Copy link
Member Author

@lutangar Yep, cleaning up and adapting the background store to answer my FIXME comment

Copy link
Member

@lutangar lutangar 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 we should maintain a list of actives notices for each tab.

I agree.

Otherwise LGTM but we should see it in action to verify everything.

@JalilArfaoui
Copy link
Member Author

JalilArfaoui commented Oct 4, 2019

@lutangar I spoke with @MaartenLMEM and @bmenant we keep going with this dirty version for now, and I open another PR for the refactoring.

@JalilArfaoui JalilArfaoui merged commit 431caa8 into develop Oct 4, 2019
@JalilArfaoui JalilArfaoui deleted the fix/closing_does_not_mark_as_read branch October 4, 2019 09:27
@lutangar
Copy link
Member

lutangar commented Oct 4, 2019

@JalilArfaoui it was approved anyway

bmenant added a commit that referenced this pull request Oct 11, 2019
# [3.0.0](v2.3.2...v3.0.0) (2019-10-11)

### Bug Fixes

* **account:** fix broken links ([#396](#396)) ([9228996](9228996))
* **AddNoticeButton:** missing uppercase ([b8ad7a8](b8ad7a8))
* **AddNoticeButton:** removed visited state, not useful in our situation ([27c4594](27c4594))
* **BackgroundButton:** remove change of color on disabled hover state ([7142d5b](7142d5b))
* **BackgroundButton story:** added missing disabled argument ([e3c6bc0](e3c6bc0))
* **badge:** bad (empty) badge update on unfold ([#423](#423)) ([8dc14ec](8dc14ec))
* **BottomLine:** removed it from SuggestionScreen, moved it to a molecule, centered button ([c4cfc79](c4cfc79))
* **BottomLine Button:** centered ([5a93743](5a93743))
* **BubbleIcon:** styled components caveat something ([bbb77e0](bbb77e0))
* **Button disabled loading hover:** changed color ([32cf024](32cf024))
* **Buttons:** display in extension ([abbe0ed](abbe0ed))
* **Buttons:** restored border radius ([d21f99e](d21f99e))
* **ContibutorButton:** restored default behaviour ([be91928](be91928))
* **ContibutorCompact:** add stat name ([de428ad](de428ad))
* **contribute:** add missing link on success page ([31e44cd](31e44cd))
* **contribution:**  still uses JotForm ([#428](#428)) ([20c0ba4](20c0ba4))
* **contribution:** decorate UI with Router first ([4ccb479](4ccb479))
* **contribution:** fix global error not showing up anymore on the preview screen if submission failed ([1b871fc](1b871fc))
* **contribution:** show contributor name on preview screen ([9c74e99](9c74e99))
* **contribution:** URL is wrong or missing in contribution email ([#449](#449)) ([7ae5538](7ae5538))
* **ContributorBorderButton:** changed color ([c4489cd](c4489cd))
* **contributorButton:** fixed sized and hover ([55f0bae](55f0bae))
* **ContributorButton:** changes subscribed and not subscribed display ([9757d20](9757d20))
* **ContributorCompact:** display only number of Bulles ([1b831a1](1b831a1))
* **ContributorLarge:** display only number of Bulles ([7a35598](7a35598))
* **ContributorLarge stat:** added Bubble icon ([4adccd2](4adccd2))
* **ContributorNav:** remove invite a friend button ([6d5bef1](6d5bef1))
* **create:** inactive create button on empty notice list ([14d356c](14d356c))
* **csp:** allow img from lmem.net and font from self and data ([1b3e6b6](1b3e6b6))
* **Deleted Notice:** aligned right now ([#393](#393)) ([9c9782b](9c9782b))
* **dismiss:** fix link to detailed Notice being triggered ([fd0d913](fd0d913))
* **Error screen:** center button ([7c43045](7c43045))
* **ExamplesScreen:** changed text ([43f1a79](43f1a79))
* **extension:** change About text ([f3737f1](f3737f1))
* **extension:** change About text ([41dde5f](41dde5f))
* **extension:** change About text ([0cfc4df](0cfc4df))
* **extension:** changed About links ([76959ba](76959ba))
* **extension:** changed About links ([b8d17fd](b8d17fd))
* **extension:** changed About links ([195e676](195e676))
* **extension:** deleted useless About text ([5bb946c](5bb946c))
* **extension:** deleted useless About text ([267f948](267f948))
* **extension:** deleted useless About text ([f8ead19](f8ead19))
* **extension:** replaced all borderButtons with background ones ([f93ca5f](f93ca5f))
* **extension about:** added missing space ([89a2ce5](89a2ce5))
* **extension menu:** removed filters link ([ad907b2](ad907b2))
* **extension notice preview:** added notice details display ([2d8f14f](2d8f14f))
* **Feedbacks:** added back missing margin between buttons ([41548f7](41548f7))
* **firefox:** catch sentry init error in content script ([adebbbb](adebbbb))
* **go-back-button:** fix go back button presents on first render ([b40e700](b40e700)), closes [#253](#253)
* **help:** remove contact form and add missing links ([ae94aee](ae94aee))
* **help:** update support address ([0f23b1e](0f23b1e))
* **iframe:** hide iframe when content closed ([d4102cc](d4102cc))
* **installation-details:** wait for store rehydration before dispatching installation details ([d47e690](d47e690))
* **ListContainer:** create container specifically for the list ([ef62c41](ef62c41))
* **loading:** show loading IN the Notification UI ([7e81db1](7e81db1))
* **Loading states in Buttons:** renamed files, changed svg fill ([67d6635](67d6635))
* **Logo margin:** added space under the logo ([e50a1a0](e50a1a0))
* **LogoBeta:** change logo in onboarding and subscription ([8efe995](8efe995))
* **manifest:** description is too long ([#448](#448)) ([d6350b3](d6350b3))
* **manifest:** rename shortname to short_name ([ad67ec0](ad67ec0))
* **Message:** removed margin for <p> ([#394](#394)) ([dc97833](dc97833))
* **message-excerpt:** raise number of chars shown to spread over three lines ([a3a80b2](a3a80b2)), closes [#310](#310)
* **notice list:** removed useless code ([#438](#438)) ([0119d99](0119d99))
* **notice-faker:** set a random id number (a cheap one, may uses an uid instead) ([dd36809](dd36809))
* **notification:** missing loading when already mounted ([#395](#395)) ([636de89](636de89))
* **Onbarding:** font size and margin ([e3db53a](e3db53a))
* **onboarding:** added margins in Examples and around stepper ([49782ca](49782ca))
* **onboarding:** center TOS form not validated and validated text ([1a9ed0d](1a9ed0d))
* **onboarding:** changed hover on BackgroundButton ([bc1c787](bc1c787))
* **onboarding:** changed stepper width ([1a7f4cd](1a7f4cd))
* **onboarding:** changed Subscribe BottomLine button color ([122c75e](122c75e))
* **onboarding:** corrected texts ([6e97c5d](6e97c5d))
* **onboarding:** fix wrapper typings ([2312f98](2312f98))
* **onboarding:** if it's an update and the ToS are accepted and the user has subscriptions, don't trigger onboarding process ([a4c18b6](a4c18b6))
* **onboarding:** inclusive writing ([4d3bc44](4d3bc44))
* **onboarding:** LMEM->Bulles, removed Beta in logo ([729a75b](729a75b))
* **onboarding:** Options screen doesn't update subscriptions ([c6cf6ed](c6cf6ed))
* **onboarding:** prevent duplicates in subscribe screen ([9094e5e](9094e5e))
* **onboarding:** removed duplicate code for button ([8321789](8321789))
* **onboarding:** wrong path when install with already accepted tos ([fd95e3b](fd95e3b))
* **onboarding examples:** corrected typo ([#429](#429)) ([14b994a](14b994a))
* **Onboarding examples:** changed content for LMEMToBulles ([b53cff6](b53cff6))
* **onboarding LMEMToBulles:** added arrow between logos ([35d6770](35d6770))
* **onboarding screen1:** changed text, added strong ([#425](#425)) ([7e80e7e](7e80e7e))
* **onboarding screen2:** center intro ([3c753d2](3c753d2))
* **onboarding screen2:** corrected top padding ([6f25566](6f25566))
* **onboarding screen2:** removed fixed header ([7526a03](7526a03))
* **onboarding subscribe screen:** changed spaces in bottom bar ([7ff748f](7ff748f))
* **onboarding subscribe screen:** corrected if condition ([0bade84](0bade84))
* **onboarding subscribe screen:** display as asked ([6cf5e43](6cf5e43))
* **onboarding subscribe screen:** tried to correct TS problem ([2c15ca3](2c15ca3))
* **Onboarding text:** changed text and links color ([e3a4630](e3a4630))
* **onboarding TOS:** added margin to headings ([fc8b5c4](fc8b5c4))
* **onboarding TOS:** changed text and font-size ([4e8a8f7](4e8a8f7))
* **Onboarding TOS:** missing stepper and bold in LMEMToBulles ([ce38fd5](ce38fd5))
* **OpenButton:** fix style ([a3d1964](a3d1964))
* **options:** add ut8 meta ([e8b9d69](e8b9d69))
* **options:** browser icon bypasses TOS acceptation ([#437](#437)) ([283e9be](283e9be))
* **options:** browser icon bypasses TOS acceptation ([#439](#439)) ([be25714](be25714))
* **Package.json:** change commit lint order ([#381](#381)) ([f8d5349](f8d5349))
* **screens:** reduced top and bottom margin ([#431](#431)) ([e31c518](e31c518))
* **sendinblue:** handle all 2xx codes ([74b8237](74b8237))
* **sentry:** Wrong project DSN ([39151df](39151df))
* **stories:** fix AddNoticeLink story ([73cf5b6](73cf5b6))
* **storybook:** form store missing ([#451](#451)) ([69fd20e](69fd20e))
* **subscribe:** suggestions shows already subscribed contributors ([5da194d](5da194d))
* **subscribe bottom bar:** added opacity ([#432](#432)) ([089e390](089e390))
* **subscriptions:** add go to suggestion on "See more" button ([2544960](2544960))
* **subscriptions:** fix "Manage" subscriptions button that would'nt open subscriptions options ([2694aad](2694aad))
* **subscriptions:** show example link on subscriptions ([8ffdf72](8ffdf72))
* **Subscriptions Screen:** better grid display ([4307321](4307321))
* **subscriptions suggestions:** added margins ([d9f8a43](d9f8a43))
* **SubscriptionScreens:** fix few bugs ([c21623e](c21623e))
* **suggestions:** add no suggestions message ([eec1c53](eec1c53))
* fix ESLint errors ([5d8788b](5d8788b))
* **TabsNav:** added a missing semicolon 🙄 ([b6fbbec](b6fbbec))
* added margin left to button ([98ed7aa](98ed7aa))
* closing does not not mark current notices as read ([#408](#408)) ([431caa8](431caa8))
* **TOS:** change font size ([992e192](992e192))
* **TOS height:** changed margins, font-size ([#436](#436)) ([a18f8ec](a18f8ec))
* **typescript:** fix button usage in a styled component CSS ([523eafb](523eafb))
* **ui:** Notice marked read when UI closed ([#346](#346)) ([c2cc908](c2cc908))
* remove duplicate message listening ([#352](#352)) ([fac235b](fac235b))
* **UserName:** adapt it in Contributor Compact ([4243c61](4243c61))
* **UserName display:** force it to be on one line ([54a23f1](54a23f1))

### chore

* change technical branding to Bulles ([e3e7de3](e3e7de3))
* fix Bulles version number to 3.0.0 ([e8fa6bb](e8fa6bb))

### Features

* **browser-action:** set grey icon when there are no notices ([c0e2a49](c0e2a49))
* **bulles-update:** service notice style ([#391](#391)) ([75d4f9b](75d4f9b))
* **contribute:** remove link to the page contributed to ([c6ea836](c6ea836))
* **contribution:** setup sendinblue for contribution forward ([63ff548](63ff548))
* **contribution-form:** find global error manually depending on each fields state ([ed2de7b](ed2de7b))
* **contribution-form:** show form errors when a field is touched and invalid ([cd127cb](cd127cb))
* **contributions:** changed intentions by avatar ([#364](#364)) ([d55ac98](d55ac98))
* **contributor-description:** allow Html in contributor description ([6a500f3](6a500f3))
* **ContributorCard:** added link to see examples ([39a6a61](39a6a61))
* **contributors:** add "missing intro" placeholder in card ([6d24e0a](6d24e0a))
* **contributors:** add subscribe/unsubscribe ([#357](#357)) ([6467ec6](6467ec6))
* **contributors:** periodically fetch contributors and send to settings ([0b7bbe0](0b7bbe0))
* **countdown:** add countdown feature on dislike and dismiss and handle side effects ([8ac93db](8ac93db))
* **Default Avatar:** added default avatar for subscriptions ([4388e77](4388e77))
* **error-handling:** capture saga errors with Sentry if enabled ([e96465d](e96465d))
* **external-messages:** add list of allowed websites for dev and all other environments ([fd73eac](fd73eac))
* **external-messages:** handle all external messages ([e365744](e365744))
* **form:** add Fields adapter ([438d250](438d250))
* **form:** add redux-form ([b4acc51](b4acc51))
* **icons:** change extension default icon to bulle, add faviconssssss on options page ([c879491](c879491))
* **notification:** suggest go back instead of refresh when error ([#397](#397)) ([cbcda3c](cbcda3c))
* **onboarding:** add Onboarding feature ([690ab8a](690ab8a)), closes [#368](#368) [#370](#370) [#374](#374)
* **onboarding:** cheap version of auto subscribe to lmem accounts in production ([91f42e8](91f42e8))
* **onboarding:** clickable example links ([#435](#435)) ([89c0563](89c0563))
* **onboarding:** save TOS acceptance on CONTINUE button ([b414fea](b414fea))
* **onboarding:** show preselected contributors in onboarding subscribe screen ([78baec4](78baec4))
* **options:** always sort contributors by nb contributions ([#427](#427)) ([902f61a](902f61a))
* **options:** declare options in manifest ([#356](#356)) ([76878f0](76878f0))
* **options:** don't show/hide subscribed/unsubscribed live ([#426](#426)) ([382b838](382b838))
* **options:** support HTML in contributors intros ([#440](#440)) ([4558c5f](4558c5f))
* **options:** use 'Lato' as default font face in options app ([4af6ca1](4af6ca1))
* remove heap analytics tracking ([a3c7b8c](a3c7b8c))
* **sentry:** do not track host page error ([ff3ee24](ff3ee24))
* **stepper:** add props to differentiate a completed step ([9b80db8](9b80db8))
* **submit-notice:** implement submit notice screen and success redirection ([130d654](130d654))
* **subscribe:** change subscribed button text when hovered ([f364573](f364573))
* **SubscribeToContributors:** added Avatar Atom ([1aefc78](1aefc78))
* **SubscribeToContributors:** added Avatar Atom ([d8bd27f](d8bd27f))
* **SubscribeToContributors:** added ContributorNav organisms ([cf6ae7a](cf6ae7a))
* **SubscribeToContributors:** added ContributorNav organisms ([53f4d16](53f4d16))
* **SubscribeToContributors:** added new button type ([ae15839](ae15839))
* **SubscribeToContributors:** added tab atom ([2543b61](2543b61))
* **SubscribeToContributors:** added user name atom ([934859a](934859a))
* **subscriptions:** change texts ([#372](#372)) ([2d78982](2d78982))
* subscribe to contributors (Merge pull request [#336](#336)) ([e3d1252](e3d1252))
* **subscriptions:** fetch matching contexts only for subscriptions ([#359](#359)) ([718b55a](718b55a))
* **subscriptions:** show message where they are no more suggestions to show in the suggestion sidebar ([ff5c40a](ff5c40a))
* **suggestions:** allow configuration of example link ([bf53a49](bf53a49))
* **transitions:** add transitions for list notices ([bf82765](bf82765))
* **ui:** withTitle HOC injects changeTitle prop to give more control ([8e53b45](8e53b45))
* add settings entrypoint ([5de91c8](5de91c8))
* created SubscriptionsScreen ([f1e1b1f](f1e1b1f))
* plug contributors to screen ([500e64d](500e64d))

### Performance Improvements

* **components:** avoid Container unmount ([#343](#343)) ([c27ab45](c27ab45))

### BREAKING CHANGES

* Releasing Bulles !
* This should trigger version 3.0.0
@bmenant
Copy link
Member

bmenant commented Oct 11, 2019

🎉 This PR is included in version 3.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

3 participants