FP-990 & FP-1255: Update `<IntroMessage>` to Use Dismissible `<SectionMessage>` #616

wesleyboar · 2022-03-24T21:40:46Z

To Do

Done

Reduce harshness of section message text and border (or create new ticket). — source
Get Design approval on proposed new colors. — request

Overview:

Use SectionMessage, not Alert, for page welcome messages.

Related Jira tickets:

Summary of Changes:

Support dismissible <SectionMessage>
Use <SectionMessage> not <Alert>.
Update documentation and tests.
Tweak CSS.

Testing Steps:

FP-990

Follow task/FP-327 - Onboarding welcome messages #191 "Testing Steps".
Confirm message design matches <SectionMessage> (see UI Patterns) not <Alert>.

FP-1225

Open UI Patterns.
Scroll to Messages & Notifications.
Dismiss the Section messages in the content—confirm they disappear.
Reload the section—confirm the dismissed Section messages have returned.

UI Photos:

Before	Initial UI	New Approved UI

Dismissal.mov

Before Click	After Click

Questions:

What is the easy way to get messages to re-appear after dismissing them? They are no longer in browser storage, so I can't just delete their statuses like I could before.

@swillrich5

@swillrich5 found and fixed this bug. I replicate the fix so I can test my FP-990 solution.

I couldn't figure out how to use `<Sectionmessage>` to achieve FP-990.

…tro-message

codecov · 2022-03-24T21:42:58Z

Codecov Report

Merging #616 (f569ac6) into main (5adcb06) will increase coverage by 2.70%.
The diff coverage is 88.23%.

❗ Current head f569ac6 differs from pull request most recent head 44a3489. Consider uploading reports for the commit 44a3489 to get more accurate results

@@            Coverage Diff             @@
##             main     #616      +/-   ##
==========================================
+ Coverage   67.51%   70.21%   +2.70%     
==========================================
  Files         408      200     -208     
  Lines       12761     5529    -7232     
  Branches     2339     1567     -772     
==========================================
- Hits         8615     3882    -4733     
+ Misses       3861     1571    -2290     
+ Partials      285       76     -209

Flag	Coverage Δ
javascript	`70.21% <88.23%> (+0.05%)`	⬆️
unittests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...components/_common/InlineMessage/InlineMessage.jsx	`100.00% <ø> (ø)`
client/src/components/_common/Message/Message.jsx	`97.22% <ø> (ø)`
...c/components/_common/IntroMessage/IntroMessage.jsx	`80.00% <66.66%> (+8.57%)`	⬆️
...mponents/_common/SectionMessage/SectionMessage.jsx	`94.11% <92.30%> (-5.89%)`	⬇️
...src/components/_common/Section/SectionMessages.jsx	`100.00% <100.00%> (ø)`
server/portal/routing.py
server/portal/__init__.py
server/portal/apps/system_creation/dryrun.py
server/portal/apps/accounts/urls.py
server/portal/apps/datafiles/utils.py
... and 203 more

…tro-message

(simple cause during migration to vite)

…tro-message

wesleyboar

Section Message looks too harsh against the Portal UI (source).

Update: I proposed new colors to Design. [Color changes were approved.]

I know I break no other styles because only Message...css used these.

wesleyboar

Notes for reviewers.

wesleyboar · 2022-04-20T16:02:43Z

client/src/components/ManageAccount/tests/ManageAccount.test.js

- expect(
- getByRole('alert', { class: /introMessageGeneral/i })
- ).toBeInTheDocument();
+ expect(getByRole('status', { name: 'message' })).toBeInTheDocument();


Explanation. Update test for <IntroMessage>, which has different aria role than did <Alert>. The aria role for any <(...)Message> is status.

wesleyboar · 2022-04-20T16:04:09Z

client/src/components/_common/IntroMessage/IntroMessage.jsx

 import { useDispatch, useSelector } from 'react-redux';

+import { SectionMessage } from '_common';


Changes on this page are for FP-990.

wesleyboar · 2022-04-20T16:05:33Z

client/src/components/_common/IntroMessage/IntroMessage.test.js

- expect(getByRole('alert')).not.toEqual(null);
+ expect(getByRole('status')).not.toEqual(null);


Explanation. Same as for <ManageAccount> test update.

wesleyboar · 2022-04-20T16:08:16Z

client/src/components/_common/Message/Message.jsx

 */
 const Message = ({
+ ariaLabel,


Explanation. Changes about ARIA label (see ff2a61e) are related to fixing <ManageAccount> test case.

wesleyboar · 2022-04-20T16:09:54Z

client/src/components/_common/Message/Message.module.scss

-.is-info .icon {
+.is-info .icon:not(.close-icon) {


Explanation. Fix same problem as in #615 (comment), but using my proposed solution.

wesleyboar · 2022-04-20T16:10:52Z

client/src/components/_common/Section/SectionMessages.jsx

@@ -54,7 +54,7 @@ function SectionMessages({
 const introMessageContent = introMessageText || MESSAGES[introMessageName];
 const introMessage = introMessageContent && (
 /* FAQ: Alternate message name allows tracking custom message dismissal */
- <IntroMessage messageName={introMessageName || introMessageText}>
+ <IntroMessage messageName={introMessageName || introMessageText} canDismiss>


Directly for FP-1255.

wesleyboar · 2022-04-20T16:11:13Z

client/src/components/_common/SectionMessage/SectionMessage.jsx

- * <Sample jsx>
+ * // basic usage
+ * <SectionMessage type="warning">Uh oh.</SectionMessage>
+ * @see _common/Message
 */
 const SectionMessage = (props) => {


Changes below on this page are for FP-1255.

client/src/components/_common/SectionMessage/SectionMessage.jsx

wesleyboar · 2022-04-20T16:29:41Z

client/src/components/_common/SectionMessage/SectionMessage.jsx

+ const autoManageVisible = props.canDismiss && props.isVisible === undefined;
+ const autoManageDismiss = props.canDismiss && props.onDismiss === undefined;


Explanation. If users want a dismissible message—i.e. they passed canDismiss—but choose to not manage that logic themselves—via isVisible and onDismiss—then this component will manage that logic automatically.

P.S. Alternative ideas are welcome.

wesleyboar · 2022-04-20T16:31:18Z

client/src/styles/settings/color.css

- --global-color-info--normal: var(--global-color-primary--x-dark);
- --global-color-info--weak: var(--global-color-primary--x-light);
+ --global-color-info--dark: var(--global-color-primary--x-dark);
+ --global-color-info--normal: var(--global-color-primary--normal);
+ --global-color-info--light: var(--global-color-primary--x-light);


Note. I slowly replace --weak colors with "standardly-named" --(x-)light and --(x-)dark.

…tro-message

rstijerina

LGTM, nice work!

rstijerina · 2022-04-25T22:25:23Z

client/src/components/_common/Message/Message.jsx

@@ -170,6 +191,7 @@ Message.propTypes = {
 type: PropTypes.oneOf(TYPES).isRequired,
 };
 Message.defaultProps = {
+ ariaLabel: 'message',


client/src/components/_common/SectionMessage/SectionMessage.jsx

duckonomy

Works well! I'll try to merge this into my FP-1375 PR (has similar parts for SectionMessages).

Left a few comments for style.

client/src/components/_common/SectionMessage/SectionMessage.jsx

duckonomy · 2022-05-03T20:46:04Z

client/src/components/_common/SectionMessage/SectionMessage.jsx

+ if (autoManageDismiss) {
+ messageProps.onDismiss = onDismiss;
+ }


Doesn't line 30 set onDismiss regardless?

Yes, nice catch. Line 30 is cruft. This conditional setting of the variable is desired. I dropped line 30 (ce6795f) and re-tested successfully.

client/src/components/_common/SectionMessage/SectionMessage.jsx

Set props undefined, instead of removing and creating two unsued vars.

duckonomy

LGTM!

wesleyboar added 7 commits September 27, 2021 12:23

FP-990: Fix bug of info Message missing X button

962115f

@swillrich5 found and fixed this bug. I replicate the fix so I can test my FP-990 solution.

FP-990: Add some examples for SectionMessage usage

b3e4bec

FP-990: <Message type=scope>, not <SectionMessage>

ef525f4

I couldn't figure out how to use `<Sectionmessage>` to achieve FP-990.

FP-990: Fix failing ManageAccount test

ff2a61e

FP-990: (CSS) Separate SectionMessage form content

17cb731

Merge branch 'main' into test/FP-990-section-message-not-alert-for-in…

405a0c5

…tro-message

Merge branch 'main' into test/FP-990-section-message-not-alert-for-in…

5d7f652

…tro-message

chore(lint): run prettier:fix

0cd1cbd

wesleyboar marked this pull request as draft April 7, 2022 16:49

wesleyboar added 7 commits April 13, 2022 10:30

Merge branch 'main' into test/FP-990-section-message-not-alert-for-in…

374f49e

…tro-message

FP-990: Fix Ability to Dimiss SectionMessages

c9c5923

Quick: Fix IntroMessage Spacing

67df4c4

(simple cause during migration to vite)

Quick: In IntroMessage Use SectionMessage

c180f22

Quick: Update Message Components JSDocs

75924aa

FP-990: More Fix Ability to Dimiss SectionMessages

2dbf2c7

Merge branch 'main' into test/FP-990-section-message-not-alert-for-in…

175f619

…tro-message

wesleyboar changed the title ~~FP-990: Update Intro Messages to Use <SectionMessage>s~~ FP-990 & FP-1255: Update <IntroMessage> to Use Dismissible <SectionMessage> Apr 19, 2022

wesleyboar marked this pull request as ready for review April 19, 2022 18:17

chore(prettier): "fix" whitespace

3078ed3

wesleyboar requested review from duckonomy and rstijerina April 19, 2022 18:27

wesleyboar commented Apr 19, 2022

View reviewed changes

wesleyboar added 2 commits April 20, 2022 10:49

FP-990: New Colors for Info Messages

bc23e3b

I know I break no other styles because only Message...css used these.

FP-1255: Fix bug that has not manifested

2b55ebf

wesleyboar commented Apr 21, 2022

View reviewed changes

Merge branch 'main' into test/FP-990-section-message-not-alert-for-in…

6a73188

…tro-message

rstijerina approved these changes Apr 27, 2022

View reviewed changes

duckonomy requested changes May 3, 2022

View reviewed changes

fix(fp-990): use const not let

8b015b5

wesleyboar added 3 commits May 3, 2022 16:37

fix(fp-990): remove leftover setting of onDismiss

ce6795f

fix(fp-990): undefine props w/out new unused vars

904765d

Set props undefined, instead of removing and creating two unsued vars.

fix(fp-990): prettier fix

44a3489

wesleyboar requested a review from duckonomy May 3, 2022 21:51

duckonomy approved these changes May 3, 2022

View reviewed changes

wesleyboar merged commit b8bf18b into main May 4, 2022

wesleyboar deleted the test/FP-990-section-message-not-alert-for-intro-message branch May 4, 2022 15:23

rstijerina changed the title ~~FP-990 & FP-1255: Update <IntroMessage> to Use Dismissible <SectionMessage>~~ FP-990 & FP-1255: Update <IntroMessage> to Use Dismissible <SectionMessage> May 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FP-990 & FP-1255: Update `<IntroMessage>` to Use Dismissible `<SectionMessage>` #616

FP-990 & FP-1255: Update `<IntroMessage>` to Use Dismissible `<SectionMessage>` #616

wesleyboar commented Mar 24, 2022 •

edited

Loading

codecov bot commented Mar 24, 2022 •

edited

Loading

wesleyboar left a comment •

edited

Loading

wesleyboar left a comment

wesleyboar Apr 20, 2022

wesleyboar Apr 20, 2022

wesleyboar Apr 20, 2022 •

edited

Loading

wesleyboar Apr 20, 2022

wesleyboar Apr 20, 2022

wesleyboar Apr 20, 2022

wesleyboar Apr 20, 2022

wesleyboar Apr 20, 2022 •

edited

Loading

wesleyboar Apr 20, 2022

rstijerina left a comment

rstijerina Apr 25, 2022

duckonomy left a comment

duckonomy May 3, 2022

wesleyboar May 3, 2022

duckonomy left a comment

		import { useDispatch, useSelector } from 'react-redux';

		import { SectionMessage } from '_common';

		expect(getByRole('alert')).not.toEqual(null);
		expect(getByRole('status')).not.toEqual(null);

		const autoManageVisible = props.canDismiss && props.isVisible === undefined;
		const autoManageDismiss = props.canDismiss && props.onDismiss === undefined;

		.is-info .icon {
		.is-info .icon:not(.close-icon) {

FP-990 & FP-1255: Update <IntroMessage> to Use Dismissible <SectionMessage> #616

FP-990 & FP-1255: Update <IntroMessage> to Use Dismissible <SectionMessage> #616

Conversation

wesleyboar commented Mar 24, 2022 • edited Loading

To Do

Overview:

Related Jira tickets:

Summary of Changes:

Testing Steps:

UI Photos:

Questions:

codecov bot commented Mar 24, 2022 • edited Loading

Codecov Report

wesleyboar left a comment • edited Loading

Choose a reason for hiding this comment

wesleyboar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wesleyboar Apr 20, 2022 • 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

wesleyboar Apr 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rstijerina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

duckonomy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

duckonomy left a comment

Choose a reason for hiding this comment

FP-990 & FP-1255: Update `<IntroMessage>` to Use Dismissible `<SectionMessage>` #616

FP-990 & FP-1255: Update `<IntroMessage>` to Use Dismissible `<SectionMessage>` #616

wesleyboar commented Mar 24, 2022 •

edited

Loading

codecov bot commented Mar 24, 2022 •

edited

Loading

wesleyboar left a comment •

edited

Loading

wesleyboar Apr 20, 2022 •

edited

Loading

wesleyboar Apr 20, 2022 •

edited

Loading