VIDCS-3321: Implement participant list toggle button, chat button, report issue button on mobile #95

behei-vonage · 2025-02-24T21:53:02Z

What is this PR doing?

This PR implements participant list toggle button, chat button, report issue button on mobile.

iPhone SE:

iPhone 14 Pro Max:

How should this be manually tested?

Checkout this branch.
Change your developer console to be a mobile device, or use a real mobile device.
Ensure that the following are showing up as you click on the "three dots" menu in the toolbar:

Change Layout
Emojis
Archiving
Report issue - if this one does not show up, ensure that you have .env file locally in frontend/ folder with VITE_ENABLE_REPORT_ISSUE set to true
Participant List
Chat

Note that all of these buttons are clickable and open what you'd expect them to. The functionality of what it is opening might not look great, that is going to be handled in subsequent PRs.

What are the relevant tickets?

A maintainer will add this ticket number.

Resolves VIDCS-3321

Checklist

[ ] Branch is based on develop (not main).
[ ] Resolves a Known Issue.
[ ] If yes, did you remove the item from the docs/KNOWN_ISSUES.md?
[ ] Resolves an item reported in Issues.
If yes, which issue? Issue Number?

behei-vonage · 2025-02-24T21:55:09Z

frontend/src/components/MeetingRoom/ArchivingToggle/ArchivingToggle.tsx

+            width: isSmallViewport ? '42px' : '48px',
+            height: isSmallViewport ? '42px' : '48px',


Note: these were needed here and in other spots to make sure that these buttons are smaller in the tiny tab ™️ devices (such as iPhone SE). I could not use isSmallViewPort directly on ToolbarButton since we have components that are dependent on it such as the toolbar overflow menu opener and the hang up button which we want to remain the same even on the tiny tab ™️ devices.

Could we add a prop to the ToolbarButton? Maybe isSmallViewport or isOverflowButton so we can apply the changes inside the component instead of adding sx props for all of these?

great idea, added!

frontend/src/components/MeetingRoom/ReportIssueButton/ReportIssueButton.tsx

cpettet

Looks good so far! Just have a few comments/questions. Let me know what you think!

frontend/src/components/MeetingRoom/ToolbarOverflowMenu/ToolbarOverflowMenu.spec.tsx

cpettet · 2025-02-24T22:55:56Z

frontend/src/components/MeetingRoom/ArchivingToggle/ArchivingToggle.tsx

+            width: isSmallViewport ? '42px' : '48px',
+            height: isSmallViewport ? '42px' : '48px',


Could we add a prop to the ToolbarButton? Maybe isSmallViewport or isOverflowButton so we can apply the changes inside the component instead of adding sx props for all of these?

cpettet

LGTM Great job!

maikthomas

Not sure why exactly but the new buttons are not aligned with the existing buttons. Will need some tweaking 🙏

behei-vonage · 2025-02-25T17:07:16Z

Not sure why exactly but the new buttons are not aligned with the existing buttons. Will need some tweaking 🙏

oh man, good eye, @maikthomas! I didn't notice this. I'll take a look 🙏

behei-vonage · 2025-02-25T18:37:35Z

frontend/src/components/MeetingRoom/ArchivingToggle/ArchivingToggle.tsx

@@ -78,6 +80,10 @@ const ArchivingToggle = (): ReactElement => {
              style={{ color: `${isRecording ? 'rgb(239 68 68)' : 'white'}` }}
            />
          }
+          sx={{
+            marginTop: isSmallViewport ? '0px' : '4px',


Note: you'll notice that we set this here instead of ToolbarButton. the reason is that we want to lower only certain buttons to make sure that they align correctly with those that are displayed only on desktop

Do we know why these ones are affected only?

these are all the ones that in the dropdown kebab menu but ToolbarButton is used across other components so it's kinda the same reason as here: #95 (comment)

cpettet · 2025-02-25T20:38:00Z

Not sure why exactly but the new buttons are not aligned with the existing buttons. Will need some tweaking 🙏

Looks like the sidebar buttons are all off, with a default style for top margin of 0px.

cpettet

Looks good so far! Just have a few comments/questions. Let me know what you think!

frontend/src/components/MeetingRoom/ParticipantListToggleButton/ParticipantListToggleButton.tsx

behei-vonage · 2025-02-25T21:56:09Z

Not sure why exactly but the new buttons are not aligned with the existing buttons. Will need some tweaking 🙏

Looks like the sidebar buttons are all off, with a default style for top margin of 0px.

Let's address these alignments in a different PR. reason being is that audio/video dropdowns need to be refactored to use ToolbarButoon instead of its own IconButton 🙏

cpettet · 2025-02-25T22:23:13Z

Let's address these alignments in a different PR. reason being is that audio/video dropdowns need to be refactored to use ToolbarButoon instead of its own IconButton 🙏

Works for me!

cpettet

LGTM Great job!

maikthomas · 2025-02-26T10:15:58Z

frontend/src/components/MeetingRoom/ArchivingToggle/ArchivingToggle.tsx

@@ -78,6 +80,10 @@ const ArchivingToggle = (): ReactElement => {
              style={{ color: `${isRecording ? 'rgb(239 68 68)' : 'white'}` }}
            />
          }
+          sx={{
+            marginTop: isSmallViewport ? '0px' : '4px',


Do we know why these ones are affected only?

maikthomas · 2025-02-26T10:16:47Z

frontend/src/components/MeetingRoom/ChatToggleButton/ChatToggleButton.tsx

          sx={{
            marginTop: '0px',
            marginRight: '0px',
          }}
          onClick={handleClick}
          icon={<ChatIcon sx={{ color: isOpen ? blue.A100 : 'white' }} />}
+          isSmallViewPort={isSmallViewport}


Can we not just access the hook value inside the ToolbarButton?

addressed here: #95 (comment) 🙏

maikthomas · 2025-02-26T10:18:05Z

frontend/src/components/MeetingRoom/ReportIssueButton/ReportIssueButton.tsx

  return (
-    <div className={`hidden ${displayOnDesktop()} px-3`}>
+    <div className="pr-3">


Hmm this is different from before px-3, will this change anything?

this doesn't seem to impact anything on desktop:

this is with px-3 in comparison on desktop:

but on mobile, switching to pr-3 helps with the gaps:

this is with px-3 in comparison:

Could you remove the right margin for the chat button? I think it's interfering with the flex layout. There's more padding on the right side versus the left.

The ChatButton did not have any right margin but I added a little bit on the layout toggle so that it aligns a bit better.

iPhone 14 Pro Max:

maikthomas · 2025-02-26T10:18:17Z

frontend/src/components/MeetingRoom/ToolbarButton/ToolbarButton.tsx

+        width: props.isSmallViewPort ? '42px' : '48px',
+        height: props.isSmallViewPort ? '42px' : '48px',


can we just use the hook here?

so it is a little tricky because if we use the hook here it'll apply it to all of the toolbar buttons and we don't want that. The reason being the kebab menu and the hand up menus are ToolbarButton components while the other ones (audio and video dropdowns) are not:

this way it'll get only applied to certain buttons

frontend/src/components/MeetingRoom/ToolbarButton/ToolbarButton.tsx

maikthomas · 2025-02-26T11:40:46Z

Some conflicts to deal with too 🙏

cpettet

Looks good so far! Just have a few comments/questions. Let me know what you think!

cpettet · 2025-02-27T16:08:09Z

frontend/src/components/MeetingRoom/ParticipantListButton/ParticipantListButton.tsx

@@ -43,12 +45,14 @@ const ParticipantListButton = ({
          overlap="circular"
        >
          <ToolbarButton
+            data-testid="participant-list-toggle"


Not a toggle!

I'm starting to suspect you don't like toggles 😂

It might be excessive, but I'm thinking about adding a lint rule for this 😂

frontend/src/components/MeetingRoom/EmojiGridButton/EmojiGridButton.tsx

frontend/src/components/MeetingRoom/Toolbar/Toolbar.spec.tsx

frontend/src/components/MeetingRoom/ToolbarOverflowButton/ToolbarOverflowButton.spec.tsx

frontend/src/components/MeetingRoom/ToolbarOverflowMenu/ToolbarOverflowMenu.spec.tsx

sonarqubecloud · 2025-03-03T16:55:40Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

cpettet

LGTM Great job!

more buttons

51ccfe9

behei-vonage self-assigned this Feb 24, 2025

behei-vonage commented Feb 24, 2025

View reviewed changes

frontend/src/components/MeetingRoom/ReportIssueButton/ReportIssueButton.tsx Show resolved Hide resolved

forgot it here

817f02b

cpettet reviewed Feb 24, 2025

View reviewed changes

behei-vonage added 2 commits February 24, 2025 19:48

feedback

e161457

forgot height

d1905e1

behei-vonage requested a review from cpettet February 25, 2025 01:52

cpettet previously approved these changes Feb 25, 2025

View reviewed changes

maikthomas requested changes Feb 25, 2025

View reviewed changes

behei-vonage and others added 2 commits February 25, 2025 11:07

Merge branch 'develop' into behei-vonage/vidcs-3321-more-buttons

e0962ac

bring it down

1fe7848

behei-vonage dismissed cpettet’s stale review via 1fe7848 February 25, 2025 18:36

behei-vonage commented Feb 25, 2025

View reviewed changes

behei-vonage requested review from maikthomas and cpettet February 25, 2025 18:41

cpettet reviewed Feb 25, 2025

View reviewed changes

frontend/src/components/MeetingRoom/ParticipantListToggleButton/ParticipantListToggleButton.tsx Outdated Show resolved Hide resolved

missed it here

5ce1373

behei-vonage requested a review from cpettet February 25, 2025 21:56

cpettet previously approved these changes Feb 25, 2025

View reviewed changes

maikthomas reviewed Feb 26, 2025

View reviewed changes

Merge branch 'develop' into behei-vonage/vidcs-3321-more-buttons

f967716

behei-vonage dismissed cpettet’s stale review via f967716 February 26, 2025 18:45

fix up da tests

7e13973

behei-vonage added 2 commits February 26, 2025 11:14

deconstruct

3bcdebf

need to align a bit better

9bba753

behei-vonage requested review from maikthomas and cpettet February 26, 2025 23:27

Merge branch 'develop' into behei-vonage/vidcs-3321-more-buttons

662c16c

cpettet reviewed Feb 27, 2025

View reviewed changes

behei-vonage and others added 5 commits March 3, 2025 07:36

Merge branch 'develop' into behei-vonage/vidcs-3321-more-buttons

fc25f04

Merge branch 'develop' into behei-vonage/vidcs-3321-more-buttons

db56dc7

participant list

afd5b01

emoji

e0314a2

Merge branch 'develop' into behei-vonage/vidcs-3321-more-buttons

f62f6e2

behei-vonage requested a review from cpettet March 3, 2025 16:59

cpettet approved these changes Mar 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VIDCS-3321: Implement participant list toggle button, chat button, report issue button on mobile #95

VIDCS-3321: Implement participant list toggle button, chat button, report issue button on mobile #95

behei-vonage commented Feb 24, 2025 •

edited

Loading

behei-vonage Feb 24, 2025

cpettet Feb 24, 2025

behei-vonage Feb 25, 2025

cpettet left a comment

cpettet Feb 24, 2025

cpettet left a comment

maikthomas left a comment

behei-vonage commented Feb 25, 2025

behei-vonage Feb 25, 2025

maikthomas Feb 26, 2025

behei-vonage Feb 26, 2025

cpettet commented Feb 25, 2025

cpettet left a comment

behei-vonage commented Feb 25, 2025

cpettet commented Feb 25, 2025

cpettet left a comment

maikthomas Feb 26, 2025

maikthomas Feb 26, 2025

behei-vonage Feb 26, 2025

maikthomas Feb 26, 2025

behei-vonage Feb 26, 2025

cpettet Feb 26, 2025

behei-vonage Feb 26, 2025

maikthomas Feb 26, 2025

behei-vonage Feb 26, 2025

maikthomas commented Feb 26, 2025

cpettet left a comment

cpettet Feb 27, 2025

behei-vonage Mar 3, 2025

cpettet Mar 3, 2025 •

edited

Loading

sonarqubecloud bot commented Mar 3, 2025

cpettet left a comment

		width: isSmallViewport ? '42px' : '48px',
		height: isSmallViewport ? '42px' : '48px',

		width: props.isSmallViewPort ? '42px' : '48px',
		height: props.isSmallViewPort ? '42px' : '48px',

VIDCS-3321: Implement participant list toggle button, chat button, report issue button on mobile #95

Are you sure you want to change the base?

VIDCS-3321: Implement participant list toggle button, chat button, report issue button on mobile #95

Conversation

behei-vonage commented Feb 24, 2025 • edited Loading

What is this PR doing?

How should this be manually tested?

What are the relevant tickets?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpettet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpettet left a comment

Choose a reason for hiding this comment

maikthomas left a comment

Choose a reason for hiding this comment

behei-vonage commented Feb 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpettet commented Feb 25, 2025

cpettet left a comment

Choose a reason for hiding this comment

behei-vonage commented Feb 25, 2025

cpettet commented Feb 25, 2025

cpettet 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

maikthomas commented Feb 26, 2025

cpettet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpettet Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 3, 2025

Quality Gate passed

cpettet left a comment

Choose a reason for hiding this comment

behei-vonage commented Feb 24, 2025 •

edited

Loading

cpettet Mar 3, 2025 •

edited

Loading