feat(extension): add midnight pre-launch settings banner LW-10398 #1156

DominikGuzei · 2024-05-13T15:06:11Z

Checklist

JIRA
Screenshots added.

Proposed solution

Please note: the CTA buttons do not have any action defined yet (as per task requirement) and will be wired-up in later tasks.

Introduce a USE_MIDNIGHT_PRELAUNCH_EVENT feature flag to hide the midnight related UI elements behind
Implement the midnight pre-launch banner in @lace/core storybook
Add the pre-launch ui setting item in the settings list (in all views)
Integrate the pre-launch banner in the settings sidebar (in extended view)

Testing

Check that no pre-launch settings items are shown when feature flag is off
With feature flag on check that the pre-launch settings item is always visible and the sidebar banner is visible in extended view in all responsive screen sizes except the smallest one.

Screenshots

video of the responsive banner sidebar banner (please note that at time of recording the settings list item was not yet activated for extended view but is in latest version):

Bildschirmaufnahme.2024-05-13.um.14.51.12.mov

DominikGuzei · 2024-05-13T15:09:19Z

...rc/ui/components/MidnightPreLaunchSettingsBanner/MidnightPreLaunchSettingsBanner.module.scss

+  line-height: 24px;
+  font-weight: 500;
+  margin-bottom: 30px;
+}


Please note that i tried to use @lace/ui typography here instead of defining it all in CSS which turned out to be more trouble than good as this text is not standard (e.g: it looks the same in both themes) and i ran into override issues when trying to change the color via css then.

DominikGuzei · 2024-05-13T15:10:30Z

...s/core/src/ui/components/MidnightPreLaunchSettingsBanner/MidnightPreLaunchSettingsBanner.tsx

+  return (
+    <Card.Outlined
+      className={styles.card}
+      style={{ '--midnight-pre-launch-banner-image': `url(${bannerImageUrl})` } as React.CSSProperties}


That was the only way I found to provide a background image via CSS variable to the component (since it didn't work to include the background-image in the compiled core component)

github-actions · 2024-05-13T15:15:30Z

Allure Report

allure-report-publisher generated test report!

smokeTests: ✅ test report for 8b83c82e

	passed	failed	skipped	flaky	total	result
Total	30	0	0	0	30	✅

VanessaPC

There is a small ts warning, but aside than that, this looks great to me!

DominikGuzei · 2024-05-14T08:56:31Z

There is a small ts warning, but aside than that, this looks great to me!

Hmm … that looks weird because i don't think this is caused by my changes in this PR?

pczeglik-iohk · 2024-05-14T09:02:47Z

...s/core/src/ui/components/MidnightPreLaunchSettingsBanner/MidnightPreLaunchSettingsBanner.tsx

+import styles from './MidnightPreLaunchSettingsBanner.module.scss';
+
+export const MidnightPreLaunchSettingsBanner = ({
+  bannerImageUrl,


I have mixed feelings about bannerImageUrl prop. In my opinion this prop would have more sense if this banner was generic one.

When I look at place where it is used I see this weird looking import import MidnightPreLaunchBannerImage from '../../../../../../../../packages/core/src/ui/assets/images/midnight-launch-event-sidebar-banner.png';

I would recommend removing this prop, or making this component generic like <SettingsBanner /> and place image in apps/browser-extension-wallet directory. The same for storybook.

The more I think about it, the more I like the idea of making this component generic one, especially that it's implementation is in core package. If we would like to keep this name MidnightPreLaunchSettingsBanner to me it would make more sense to have it in apps/browser-extension-wallet as this is not something generic and there is probably 0% chance we would like to reuse it somewhere else.

@pczeglik-iohk I had to introduce the prop because the image must be used as CSS background (so that the rounded corners of the banner are correctly respected) AND unfortunately our core package doesn't correctly bundle css images (the path is simply retained and then wrong when component is imported in the extension) - I can move the image to the extension package but then what should be shown in the core storybook (it also acts as an example)?

I agree with @DominikGuzei I think the issue is on the way we have of working and exporting images from core which should be addressed. But I think the way it was set up it's consistent with how other components currently do it.

@pczeglik-iohk sure, i can turn this into a generic component BUT it doesn't really make sense since the design is quite special in that it looks the same in light/dark theme (font colors). I'm not sure if it's the best case for a generic component tbh.

pczeglik-iohk · 2024-05-14T09:05:29Z

...s/core/src/ui/components/MidnightPreLaunchSettingsBanner/MidnightPreLaunchSettingsBanner.tsx

+
+export const MidnightPreLaunchSettingsBanner = ({
+  bannerImageUrl,
+  onMoreDetailsClick


Can we instead of onMoreDetailsClick use just onClick?

In case of button text change we won't need to change prop name to have consistency between what we see in UI and what we have in code.

what about onCtaButtonClick?

@pczeglik-iohk renamed to onCtaButtonClick and updated PR

pczeglik-iohk · 2024-05-14T09:14:36Z

packages/core/src/ui/lib/translations/en.json

@@ -250,6 +250,9 @@
  "core.infoWallet.addressCopied": "Address copied",
  "core.infoWallet.copy": "Copy",
  "core.infoWallet.handleCopied": "Handle copied",
+  "core.MidnightPreLaunchSettingsBanner.heading": "Midnight Pre-Launch Event",


If we decide to go with generic component translations should be in apps/browser-extension-wallet?

I would recommend creating dedicated namespace for all midnight related translations like browserView.midnight or just midnight.

just like you did it here https://github.com/input-output-hk/lace/pull/1156/files#diff-1ca0f877e9a3ed7324d37abb38e80309f71083101665c52a39e7c338581db781R176

@pczeglik-iohk it's not a generic component, the only reason why we develop new components in core because we haven't been able to set up storybook correctly in the extension app package 😆 if you look around you will see plenty of non-generic components in core package. Only @lace/ui is truly generic.

In both cases i followed the existing naming scheme for translations in the app and core package. I'm not sure if introducing a dedicated namespace will make things clearer now.

btw. the way we handle translations will be changed dramatically anyway (moved into a dedicated package): #1154

pczeglik-iohk

Looks good @DominikGuzei, a few changes proposed.

Let me know what do you think :)

pczeglik-iohk

Not 100% happy, but I totally understand your arguments @DominikGuzei

Code approved! Thank you @DominikGuzei

sonarqubecloud · 2024-05-21T16:33:02Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

DominikGuzei added the browser Changes to the browser application. label May 13, 2024

DominikGuzei self-assigned this May 13, 2024

github-actions bot removed the browser Changes to the browser application. label May 13, 2024

DominikGuzei commented May 13, 2024

View reviewed changes

DominikGuzei force-pushed the feat/lw-10398-settings-sidebar-midnight-event-component branch from 9b7d0cc to 29283c3 Compare May 13, 2024 15:14

DominikGuzei added the browser Changes to the browser application. label May 13, 2024

github-actions bot removed the browser Changes to the browser application. label May 13, 2024

DominikGuzei force-pushed the feat/lw-10398-settings-sidebar-midnight-event-component branch from 29283c3 to 4e25510 Compare May 13, 2024 15:55

DominikGuzei marked this pull request as ready for review May 13, 2024 15:55

DominikGuzei requested a review from a team as a code owner May 13, 2024 15:55

DominikGuzei force-pushed the feat/lw-10398-settings-sidebar-midnight-event-component branch from 4e25510 to 711980e Compare May 13, 2024 15:56

VanessaPC approved these changes May 14, 2024

View reviewed changes

pczeglik-iohk reviewed May 14, 2024

View reviewed changes

pczeglik-iohk requested changes May 14, 2024

View reviewed changes

DominikGuzei force-pushed the feat/lw-10398-settings-sidebar-midnight-event-component branch from 711980e to c6bc842 Compare May 15, 2024 10:54

pczeglik-iohk approved these changes May 15, 2024

View reviewed changes

DominikGuzei force-pushed the feat/lw-10398-settings-sidebar-midnight-event-component branch from c6bc842 to d0d7d92 Compare May 17, 2024 09:58

DominikGuzei requested a review from a team as a code owner May 17, 2024 09:58

DominikGuzei force-pushed the feat/lw-10398-settings-sidebar-midnight-event-component branch from d0d7d92 to 8db3c89 Compare May 21, 2024 10:14

feat(extension): add midnight pre-launch settings banner [LW-10398]

8b83c82

DominikGuzei force-pushed the feat/lw-10398-settings-sidebar-midnight-event-component branch from 8db3c89 to 8b83c82 Compare May 21, 2024 16:31

DominikGuzei merged commit ff562a0 into main May 21, 2024
12 checks passed

DominikGuzei deleted the feat/lw-10398-settings-sidebar-midnight-event-component branch May 21, 2024 17:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(extension): add midnight pre-launch settings banner LW-10398 #1156

feat(extension): add midnight pre-launch settings banner LW-10398 #1156

DominikGuzei commented May 13, 2024 •

edited

Loading

DominikGuzei May 13, 2024

DominikGuzei May 13, 2024

github-actions bot commented May 13, 2024 •

edited

Loading

VanessaPC left a comment

DominikGuzei commented May 14, 2024 •

edited

Loading

pczeglik-iohk May 14, 2024

pczeglik-iohk May 14, 2024

DominikGuzei May 14, 2024

VanessaPC May 14, 2024

DominikGuzei May 14, 2024

pczeglik-iohk May 14, 2024

DominikGuzei May 14, 2024

DominikGuzei May 15, 2024 •

edited

Loading

pczeglik-iohk May 14, 2024

pczeglik-iohk May 14, 2024

DominikGuzei May 14, 2024 •

edited

Loading

pczeglik-iohk left a comment

pczeglik-iohk left a comment

sonarqubecloud bot commented May 21, 2024

feat(extension): add midnight pre-launch settings banner LW-10398 #1156

feat(extension): add midnight pre-launch settings banner LW-10398 #1156

Conversation

DominikGuzei commented May 13, 2024 • edited Loading

Checklist

Proposed solution

Testing

Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 13, 2024 • edited Loading

Allure Report

VanessaPC left a comment

Choose a reason for hiding this comment

DominikGuzei commented May 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DominikGuzei May 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DominikGuzei May 14, 2024 • edited Loading

Choose a reason for hiding this comment

pczeglik-iohk left a comment

Choose a reason for hiding this comment

pczeglik-iohk left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented May 21, 2024

Quality Gate passed

DominikGuzei commented May 13, 2024 •

edited

Loading

github-actions bot commented May 13, 2024 •

edited

Loading

DominikGuzei commented May 14, 2024 •

edited

Loading

DominikGuzei May 15, 2024 •

edited

Loading

DominikGuzei May 14, 2024 •

edited

Loading