Feature/deseng695: Updated Who is Listening Widget, changed colour of Video Widget overlay logo, fixed media query for Map Widget expand link. #2598

jareth-whitney · 2024-10-02T20:52:45Z

Issue #: https://citz-gdx.atlassian.net/browse/DESENG-695
Issue #: https://citz-gdx.atlassian.net/browse/DESENG-692
Issue #: https://citz-gdx.atlassian.net/browse/DESENG-693

Description of changes:

New Who is Listening widget front end
- Implemented Figma design
- Added widget_listening table to db for widget instance data
- Added option to enter widget description text (below title, above contacts)
- Adjusted CSS to accomodate multiple viewports
- Added ARIA labels for accessibility
- Updated contact create/edit form
Video Widget
- Changed icon colour from yellow to white to accomodate company branding requests
Map Widget
- Adjusted viewport settings of expand map link to accomodate breakpoints of engagement view page

User Guide update ticket (if applicable):
N/A

…d icon color for video widget, adjusted viewport for expand map link in map widget.

…date to image picker.

codecov-commenter · 2024-10-02T21:00:12Z

Codecov Report

Attention: Patch coverage is 69.23077% with 28 lines in your changes missing coverage. Please review.

Project coverage is 75.13%. Comparing base (0b61f80) to head (bd97069).

Files with missing lines	Patch %	Lines
.../services/widgetService/ListeningService/index.tsx	43.47%	13 Missing ⚠️
...ementWidgets/WhoIsListening/WhoIsListeningForm.tsx	30.00%	7 Missing ⚠️
...ntWidgets/WhoIsListening/WhoIsListeningContext.tsx	76.19%	5 Missing ⚠️
...gagement/old-view/widgets/WhoIsListeningWidget.tsx	94.44%	2 Missing ⚠️
...ents/engagement/old-view/widgets/Map/MapWidget.tsx	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2598      +/-   ##
==========================================
- Coverage   75.17%   75.13%   -0.04%     
==========================================
  Files         612      613       +1     
  Lines       22224    22308      +84     
  Branches     1866     1882      +16     
==========================================
+ Hits        16706    16762      +56     
- Misses       5247     5275      +28     
  Partials      271      271

Flag	Coverage Δ
metweb	`64.24% <69.23%> (+0.01%)`	⬆️

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

Files with missing lines	Coverage Δ
met-api/src/met_api/models/__init__.py	`100.00% <ø> (ø)`
met-api/src/met_api/models/widget_translation.py	`98.00% <ø> (ø)`
met-api/src/met_api/resources/__init__.py	`100.00% <ø> (ø)`
met-api/src/met_api/schemas/widget_translation.py	`100.00% <ø> (ø)`
...src/met_api/services/widget_translation_service.py	`76.47% <ø> (ø)`
met-web/src/apiManager/endpoints/index.ts	`100.00% <ø> (ø)`
...agementWidgets/WhoIsListening/AddContactDrawer.tsx	`54.11% <ø> (ø)`
...agementWidgets/WhoIsListening/ContactInfoPaper.tsx	`76.47% <ø> (ø)`
...agement/old-view/widgets/Video/VideoWidgetView.tsx	`19.04% <ø> (ø)`
met-web/src/components/imageUpload/index.tsx	`95.00% <ø> (-5.00%)`	⬇️
... and 5 more

Baelx

Thanks Jareth! Looks good, could you please review all my comments. To proceed, could you:

Remove the empty labels on form controls (see my comment) or, even better, link MetLabels to form controls
See my comment about the widget listening context, just curious about this approach for this

Baelx · 2024-10-02T21:17:14Z

met-api/migrations/versions/917a911b6ca9_added_widget_listening_table_for_.py

+    sa.ForeignKeyConstraint(['widget_id'], ['widget.id'], ondelete='CASCADE'),
+    sa.PrimaryKeyConstraint('id')
+    )
+    op.add_column('widget_translation', sa.Column('listening_description', sa.Text(), nullable=True))


Thanks for remembering the translation for it!

Baelx · 2024-10-02T21:32:35Z

met-api/src/met_api/resources/widget_listening.py

+
+@cors_preflight('GET, POST')
+@API.route('')
+class Listenings(Resource):


😂 the name

I know, it reminds me of a horror movie. "The Listening".

Baelx · 2024-10-02T21:45:11Z

met-api/tests/unit/api/test_widget_listening.py

+fake = Faker()
+
+
+def test_create_listening_widget(client, jwt, session,


Thanks for adding a few tests here! Our coverage has been slowly slipping as we add a bunch of huge new features

Baelx · 2024-10-02T21:54:02Z

met-web/src/components/engagement/form/EngagementWidgets/WhoIsListening/AddContactDrawer.tsx

+                                    data-testid="contact-form/phone"
+                                    aria-label="Phone: optional."
+                                    variant="outlined"
+                                    label=" "


Thanks for improving the accessibility here. I'd recommend removing this empty label in case it causes confusion.

Baelx · 2024-10-02T22:09:20Z

...eb/src/components/engagement/form/EngagementWidgets/WhoIsListening/WhoIsListeningContext.tsx

    const [addedContacts, setAddedContacts] = useState<Contact[]>([]);
    const [loadingContacts, setLoadingContacts] = useState(true);

+    if (!widget) {


Just curious about this part - why did we need to check if there was no widget and then return a different context provider?

The widget is passed down as a prop, and if it's missing, it just displays a blank widget. That's the way it was designed before. We could change it if needed. I have moved the useEffect to the top of the component to avoid the errors.

Just to confirm what we discussed over Teams, it's a useContext, not a prop, that it's passed down from. I know that I didn't create this code, but I couldn't find it on main, so I removed it, made some additional conditions, and tested again. I'm having no errors or issues now, so this code has been removed.

Baelx · 2024-10-02T22:15:29Z

met-web/src/components/engagement/form/EngagementWidgets/WhoIsListening/WhoIsListeningForm.tsx

+                        setListeningWidget({ ...listeningWidget, description: e.target.value });
+                    }}
+                    label=" "
+                    aria-label="Description: optional."


No need to change things now, but in the future I'd advise that we use the actual label. If you have multiple elements that label a control, you can use aria-labelledby="id1 id2" where the IDs correspond to each labelling element. From WCAG: "Whenever possible, use the label element to associate text with form elements explicitly."

https://www.w3.org/WAI/tutorials/forms/labels/

Baelx · 2024-10-02T22:17:54Z

met-web/src/components/engagement/old-view/widgets/WhoIsListeningWidget.tsx

+            setIsLoading(false);
+        } catch (error) {
+            setIsLoading(false);
+            console.log(error);


Even though we may retire this code at some point soon, I'd still recommend you use console.error and give the log a descriptive label to help us locate it in the code if we're troubleshooting.

Suggested change

console.log(error);

console.error('Error loading who is listening widget.', error);

Yes sorry, that was just for my benefit, it's been removed.

Baelx · 2024-10-02T22:24:57Z

met-web/src/components/engagement/old-view/widgets/WhoIsListeningWidget.tsx

-                                <Grid container justifyContent={{ xs: 'center', md: 'flex-start' }} item xs={12}>
-                                    <BodyText>{contact.title}</BodyText>
+                                <Grid justifyContent={{ xs: 'center', md: 'flex-start' }} xs={12}>
+                                    <BodyText sx={contactTitleStyles} aria-label="Contact job title:">


Hmm, I'm not necessarily against this but just curious on why you added the aria label? Is it clear to sighted users that this is a job title, but not to non-sighted users?

I guess it depends how you think it should be read. To me, it should be like name: John Smith, contact job title: minister of mining, etc. But it could also just read the fields, like John Smith, Minister of Mining.

I changed it to the shorter way.

… form for better legibility.

sonarqubecloud · 2024-10-03T00:12:22Z

Quality Gate passed

Issues
9 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

Baelx

Thanks for the changes! Approved

jareth-whitney added 3 commits October 2, 2024 10:53

feature/deseng695: Implemented Who Is Listening widget design, change…

763e5ae

…d icon color for video widget, adjusted viewport for expand map link in map widget.

Merge branch 'main' into feature/deseng695

f11514f

feature/deseng695: Updated contact create/edit form, minor styling up…

34a7790

…date to image picker.

jareth-whitney requested review from Baelx and NatSquared October 2, 2024 20:52

Baelx requested changes Oct 2, 2024

View reviewed changes

jareth-whitney added 2 commits October 2, 2024 16:57

feature/deseng695: Made changes as per Alex, updated Who is Listening…

bd97069

… form for better legibility.

feature/deseng695: Removed blank labels from contact form.

bf75d52

Baelx approved these changes Oct 3, 2024

View reviewed changes

jareth-whitney merged commit bb89f8d into main Oct 3, 2024
12 of 15 checks passed

jareth-whitney deleted the feature/deseng695 branch October 3, 2024 00:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/deseng695: Updated Who is Listening Widget, changed colour of Video Widget overlay logo, fixed media query for Map Widget expand link. #2598

Feature/deseng695: Updated Who is Listening Widget, changed colour of Video Widget overlay logo, fixed media query for Map Widget expand link. #2598

jareth-whitney commented Oct 2, 2024

codecov-commenter commented Oct 2, 2024 •

edited

Loading

Baelx left a comment

Baelx Oct 2, 2024

Baelx Oct 2, 2024

jareth-whitney Oct 2, 2024

Baelx Oct 2, 2024

Baelx Oct 2, 2024

Baelx Oct 2, 2024

jareth-whitney Oct 2, 2024

jareth-whitney Oct 2, 2024

Baelx Oct 2, 2024

Baelx Oct 2, 2024

jareth-whitney Oct 2, 2024

Baelx Oct 2, 2024

jareth-whitney Oct 2, 2024

jareth-whitney Oct 2, 2024

sonarqubecloud bot commented Oct 3, 2024

Baelx left a comment

		fake = Faker()


		def test_create_listening_widget(client, jwt, session,

	console.log(error);
	console.error('Error loading who is listening widget.', error);

Feature/deseng695: Updated Who is Listening Widget, changed colour of Video Widget overlay logo, fixed media query for Map Widget expand link. #2598

Feature/deseng695: Updated Who is Listening Widget, changed colour of Video Widget overlay logo, fixed media query for Map Widget expand link. #2598

Conversation

jareth-whitney commented Oct 2, 2024

codecov-commenter commented Oct 2, 2024 • edited Loading

Codecov Report

Baelx left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Oct 3, 2024

Quality Gate passed

Baelx left a comment

Choose a reason for hiding this comment

codecov-commenter commented Oct 2, 2024 •

edited

Loading