Implement new release banner - Closes #2144 #2300

massao · 2019-08-01T08:33:23Z

What issue have I solved?

#2144

How have I implemented/fixed it?

Refactor flashMessage toolbox component to be more generic;
Created base header to render signInHeader or topBar component;
Created flashMessageHolder to be easier to render new flash messages;
Pass data from the IPC to new release banner;

The release summary is fetched from the release note and consists of the content of the first html tag.

The following items will be done in a followup ticket #2317 .

Open modal with IPC data through release banner;
Start update through click on dialog;
Show toaster that update started;

How has this been tested?

Change version on package.json, build the application an run with electront o see that the banner shows up.

Review checklist

The PR follows our Test guide
The PR follows our CSS guide

slaweet · 2019-08-01T08:51:49Z

src/components/toolbox/flashMessage/flashMessage.js

 };

+FlashMessage.displayName = 'FlashMessage';
+FlashMessage.Text = Text;
+FlashMessage.Button = Button;


Awesome 🏆

slaweet

🐛 The release notes from Github can contain HTML that is escaped (see screenshot)
🐛 In the release notes, the first line is the title of the first section (usually "Fixed bugs" or "Implemented enhancements"), It should IMO show the second line. (see "body" in https://api.github.com/repos/LiskHQ/lisk-hub/releases)
🐛 "Read more" seems slightly vertically miss-aligned with the text.
❓ On the dark splash screen it looks a bit weird, maybe bottom border would help. Please check with design team.

Otherwise, it works well. I'm still going through the code...

massao · 2019-08-05T08:56:19Z

🐛 The release notes from Github can contain HTML that is escaped (see screenshot)

I'm checking into it.

🐛 In the release notes, the first line is the title of the first section (usually "Fixed bugs" or "Implemented enhancements"), It should IMO show the second line. (see "body" in https://api.github.com/repos/LiskHQ/lisk-hub/releases)

By what I discussed with @reyraa for future releases we are going to have the first line being a heading with the summary. But we can check other alternatives with him, if needed.

🐛 "Read more" seems slightly vertically miss-aligned with the text.

This seems to be because the button has a font-size: 15px; and the text has font-size: 13px, because it's using display: flex; align-items: center; so it should be ok.

❓ On the dark splash screen it looks a bit weird, maybe bottom border would help. Please check with design team.

Will check and update this comment with the conclusion.
Design team will update invision with one version of the splashscreen with the banner. https://projects.invisionapp.com/d/main#/console/17570736/377372026/preview

slaweet · 2019-08-05T08:58:15Z

🐛 Account initialization is not displayed anymore (I saw it in the HTML, but CSS makes it not visible somehow)

slaweet · 2019-08-05T09:04:00Z

🐛 In the release notes, the first line is the title of the first section (usually "Fixed bugs" or "Implemented enhancements"), It should IMO show the second line. (see "body" in https://api.github.com/repos/LiskHQ/lisk-hub/releases)

By what I discussed with @reyraa for future releases we are going to have the first line being a heading with the summary. But we can check other alternatives with him, if needed.

The first line with summary sounds good. Will there be some formatting, like bold or H1-6 @reyraa ? IMO it makes sense to have the summary somehow highlighted on Github as one of the most important parts. Can we already do it for the latest release?
https://github.com/LiskHQ/lisk-hub/releases/tag/v1.19.1 So that we can test what does it look like now and not after the release.

src/utils/newRelease.js

src/components/newReleaseMessage/newReleaseMessage.js

reyraa · 2019-08-05T12:33:23Z

@slaweet @massao Yes, let's use a heading for the summary so it's easier read and we can grab the summary easily.

reyraa · 2019-08-05T13:21:12Z

@massao I jazzed up this release note so you can test easily.
https://github.com/LiskHQ/lisk-hub/releases/tag/v1.19.1
Let me know if it works for you.

slaweet · 2019-08-05T13:44:15Z

🐛 Currently, it displays only the "Release goal" text. Do we want to change the code or the release notes format?

slaweet · 2019-08-05T13:45:48Z

When I tried to change it to display the actual text, I found out 🐛 it doesn't wrap the text on multiple lines if too long (or small screen)

src/utils/htmlParser.js

massao · 2019-08-05T14:39:15Z

🐛 Currently, it displays only the "Release goal" text. Do we want to change the code or the release notes format?

For now I set the releaseSummary as being the first h4 on the release notes.

When I tried to change it to display the actual text, I found out 🐛 it doesn't wrap the text on multiple lines if too long (or small screen)

Fixed the wrapping issue, but had to disable show/hide animations, as it was relying on a specific height.

src/utils/newRelease.test.js

slaweet

Thank you Massao, good job.

Efefefef

Great 👍

massao self-assigned this Aug 1, 2019

massao added this to Pull Requests in Version 1.20.0 via automation Aug 1, 2019

slaweet reviewed Aug 1, 2019

View reviewed changes

massao force-pushed the 2144-implement-new-release-banner branch 3 times, most recently from 11879de to cf5a04e Compare August 2, 2019 13:17

♻️ Refactor flashMessage toolbox component to be more generic

c51fd1d

massao force-pushed the 2144-implement-new-release-banner branch from bb1dc8b to b2d717c Compare August 2, 2019 13:53

massao added 9 commits August 2, 2019 17:35

♻️ Create header component to render correct one in each route

53b1191

♻️ Add flashMessageHolder singleton to be easier to display messages

497ebcc

💅 Fix css issues on electron after updating app component

73f9b24

♻️ Update flashMessageHolder to deleteMessage on dismiss

546619e

♻️ Update newReleaseMessage component to correct styling

c9eee2b

♻️ Add newRelease util to get info form ipc and render flashMessage

cf0b88b

♻️ Adjust autoUpdater to send information to react

6fa4eaa

♻️ Update generated strings

27e588a

✅ Add FlashMessageHolder unit tests

767524b

massao force-pushed the 2144-implement-new-release-banner branch from b2d717c to 981d1ea Compare August 3, 2019 13:47

♻️ Adjust newRelease util and message and add unit test

8e29375

massao force-pushed the 2144-implement-new-release-banner branch from 981d1ea to 8e29375 Compare August 3, 2019 14:04

massao changed the title ~~Implement new release banner and dialog - Closes #2144~~ Implement new release banner Aug 5, 2019

massao marked this pull request as ready for review August 5, 2019 07:45

massao requested a review from slaweet August 5, 2019 07:45

slaweet suggested changes Aug 5, 2019

View reviewed changes

src/utils/newRelease.js Outdated Show resolved Hide resolved

src/components/newReleaseMessage/newReleaseMessage.js Outdated Show resolved Hide resolved

massao added 2 commits August 5, 2019 11:15

♻️ Fix initialization banner not showing up.

65a4b11

✅ Improve FlashMessageHolder unit test

07bf2ac

♻️ Add border-bottom to flashMessage

818b4de

massao force-pushed the 2144-implement-new-release-banner branch from 980b57e to 818b4de Compare August 5, 2019 12:10

♻️ Add htmlParser util to handle parsing HTML inside strings

b1da817

massao requested a review from slaweet August 5, 2019 13:16

massao added 2 commits August 5, 2019 16:09

♻️ Update regex of releaseSummary

153e3ac

♻️ Improve htmlParser to handle text before tag openning

88347c9

slaweet reviewed Aug 5, 2019

View reviewed changes

src/utils/htmlParser.js Outdated Show resolved Hide resolved

massao added 2 commits August 5, 2019 16:35

♻️ Fix issue with resizing on flashMessage

6302967

♻️ Renamed parser util to htmlStringToReact to be more descriptive

67594a5

slaweet reviewed Aug 5, 2019

View reviewed changes

src/utils/newRelease.test.js Show resolved Hide resolved

massao added 2 commits August 5, 2019 17:01

💅 Remove unecessary rules from offlineWrapper css

68ff5cb

✅ Adjust newRelease to delete ipc from window after each test

a50256c

massao force-pushed the 2144-implement-new-release-banner branch from 8d392a5 to a50256c Compare August 5, 2019 15:25

💅 Adjust css pages centered vertically after change on header

62829a5

slaweet approved these changes Aug 5, 2019

View reviewed changes

slaweet requested a review from Efefefef August 5, 2019 16:00

reyraa added this to the Sprint 6 milestone Aug 6, 2019

slaweet requested a review from yasharAyari August 6, 2019 07:51

massao changed the title ~~Implement new release banner~~ Implement new release banner - Closes #2144 Aug 6, 2019

Efefefef approved these changes Aug 7, 2019

View reviewed changes

Efefefef added the ready label Aug 7, 2019

massao removed the request for review from yasharAyari August 7, 2019 12:05

massao merged commit f82e042 into development Aug 7, 2019

Version 1.20.0 automation moved this from Pull Requests to Merged Pull Requests Aug 7, 2019

massao deleted the 2144-implement-new-release-banner branch August 7, 2019 12:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement new release banner - Closes #2144 #2300

Implement new release banner - Closes #2144 #2300

massao commented Aug 1, 2019 •

edited

slaweet Aug 1, 2019

slaweet left a comment •

edited

massao commented Aug 5, 2019 •

edited

slaweet commented Aug 5, 2019

slaweet commented Aug 5, 2019

reyraa commented Aug 5, 2019

reyraa commented Aug 5, 2019

slaweet commented Aug 5, 2019

slaweet commented Aug 5, 2019

massao commented Aug 5, 2019

slaweet left a comment

Efefefef left a comment

Implement new release banner - Closes #2144 #2300

Implement new release banner - Closes #2144 #2300

Conversation

massao commented Aug 1, 2019 • edited

What issue have I solved?

How have I implemented/fixed it?

How has this been tested?

Review checklist

slaweet Aug 1, 2019

Choose a reason for hiding this comment

slaweet left a comment • edited

Choose a reason for hiding this comment

massao commented Aug 5, 2019 • edited

slaweet commented Aug 5, 2019

slaweet commented Aug 5, 2019

reyraa commented Aug 5, 2019

reyraa commented Aug 5, 2019

slaweet commented Aug 5, 2019

slaweet commented Aug 5, 2019

massao commented Aug 5, 2019

slaweet left a comment

Choose a reason for hiding this comment

Efefefef left a comment

Choose a reason for hiding this comment

massao commented Aug 1, 2019 •

edited

slaweet left a comment •

edited

massao commented Aug 5, 2019 •

edited