Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Refactor Calls components into smaller pieces for future Widgets work #6475

Merged
merged 25 commits into from
Aug 6, 2021

Conversation

Palid
Copy link
Contributor

@Palid Palid commented Jul 26, 2021

This pull request only refactors Calls into a slightly smaller, reusable piece which will make reusing it in the future for Widgets much easier.

@Palid Palid added T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements labels Jul 26, 2021
@Palid Palid force-pushed the jitsi-picture-in-picture branch from 56f9430 to b19ed64 Compare July 26, 2021 12:34
@germain-gg germain-gg marked this pull request as draft July 26, 2021 12:36
@dbkr dbkr removed the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Jul 26, 2021
@dbkr
Copy link
Member

dbkr commented Jul 26, 2021

Have removed T-Enhancement: I think this probably falls into bugfix, but it definitely shouldn't be both :)

Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done on this first PR, I've added a couple of comments.

I believe this is already part of your backlog but you should probably get CallPreview to extend PictureInPictureDragger

src/components/views/elements/PersistedElement.js Outdated Show resolved Hide resolved
src/components/views/voip/CallView/CallViewHeader.tsx Outdated Show resolved Hide resolved
src/components/views/voip/PictureInPictureDragger.tsx Outdated Show resolved Hide resolved
src/components/views/voip/PictureInPictureDragger.tsx Outdated Show resolved Hide resolved
src/components/views/voip/PictureInPictureDragger.tsx Outdated Show resolved Hide resolved
@Palid Palid force-pushed the jitsi-picture-in-picture branch from 2bf64f9 to f4061f0 Compare July 27, 2021 08:24
@Palid Palid force-pushed the jitsi-picture-in-picture branch 2 times, most recently from 2f5b87a to 59b6f56 Compare August 3, 2021 10:40
@Palid Palid removed the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Aug 3, 2021
@Palid Palid requested a review from germain-gg August 3, 2021 10:41
@Palid Palid force-pushed the jitsi-picture-in-picture branch from 59b6f56 to 9c89a13 Compare August 3, 2021 10:42
@Palid Palid changed the title DRAFT: Jitsi picture in picture Refactor Calls components into smaller pieces for future Widgets work Aug 3, 2021
@Palid Palid force-pushed the jitsi-picture-in-picture branch from 9c89a13 to ae411b9 Compare August 3, 2021 10:58
@Palid Palid requested a review from SimonBrandner August 3, 2021 11:26
@Palid
Copy link
Contributor Author

Palid commented Aug 3, 2021

@SimonBrandner here's the small refactor that we can probably improve upon in the future when normalizing the widgets rendering.

Copy link
Contributor

@SimonBrandner SimonBrandner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks generally quite great! Just a few smaller things, also the indentation is super inconsistent/spacing

src/components/views/voip/CallView/CallViewHeader.tsx Outdated Show resolved Hide resolved
src/components/views/voip/CallView/CallViewHeader.tsx Outdated Show resolved Hide resolved
src/components/views/voip/PictureInPictureDragger.tsx Outdated Show resolved Hide resolved
src/components/views/voip/CallView/CallViewHeader.tsx Outdated Show resolved Hide resolved
src/components/views/voip/CallView/CallViewHeader.tsx Outdated Show resolved Hide resolved
src/components/views/voip/CallView/CallViewHeader.tsx Outdated Show resolved Hide resolved
@Palid Palid force-pushed the jitsi-picture-in-picture branch from 6f29b09 to b18d03b Compare August 3, 2021 13:21
@Palid Palid requested a review from dbkr August 3, 2021 16:42
Palid added 3 commits August 6, 2021 13:25
…icture

* origin/develop: (100 commits)
  Add comments to isRegionalIndicator
  Stop voice messages that are playing when starting a recording
  Properly set style attribute on shared usercontent iframe
  Fix in-call context menus when in PiP mode (#6552)
  Extract tooltipYOffset to a const
  Increase yOffset by 4px away
  i18n
  Post-merge conflict resolution and improve alignment of tooltips
  Fix image & blurhash info when skipping thumbnail due to thresholds
  Skip sending a thumbnail if it is not a sufficient saving over the original
  Null guard space inviter to prevent the app exploding
  Remove seams from pin icon
  Appease Jest
  Fix worklet reference for new webpack pipeline
  i18n
  Update copy
  Fix wrong cursor being used in PiP
  Fix voice feed cut-off
  Use flex-start as it has more universal support
  Wrap cases in {}
  ...
@Palid Palid requested a review from a team as a code owner August 6, 2021 11:28
@Palid Palid requested a review from SimonBrandner August 6, 2021 11:29
src/components/views/voip/PictureInPictureDragger.tsx Outdated Show resolved Hide resolved
src/components/views/voip/PictureInPictureDragger.tsx Outdated Show resolved Hide resolved
res/css/views/voip/_CallViewHeader.scss Outdated Show resolved Hide resolved
@Palid Palid requested a review from SimonBrandner August 6, 2021 14:53
Copy link
Contributor

@SimonBrandner SimonBrandner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Palid Palid disabled auto-merge August 6, 2021 15:44
@Palid Palid enabled auto-merge August 6, 2021 15:48
@Palid Palid merged commit b6f7c4f into develop Aug 6, 2021
@Palid Palid deleted the jitsi-picture-in-picture branch August 6, 2021 16:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants