WIP: Implement `Knocking` #9093

ankur12-1610 · 2022-07-23T20:35:10Z

Checklist

Tests written for new code (and old code if feasible)
Linter and other CI checks pass
Sign-off given on the changes (see CONTRIBUTING.md)

Demo for Creating Room:

Knocking-demo.mp4

Demo for Room Settings:

demo-room-settings.mp4

Related PR: vector-im/#22926

This PR currently has none of the required changelog labels.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

SimonBrandner

Looking great! Just a few nits mainly regarding the desings

src/i18n/strings/en_EN.json

src/components/views/elements/JoinRuleDropdown.tsx

src/components/views/dialogs/CreateRoomDialog.tsx

ankur12-1610 · 2022-07-24T11:04:49Z

Looking great! Just a few nits mainly regarding the desings

@SimonBrandner I've resolved nits and added one more commit, also I;ve updated the video can you please look into it?

SimonBrandner

Looking great

SimonBrandner

Looking great!

SimonBrandner · 2022-08-01T07:08:02Z

src/components/views/settings/JoinRuleSettings.tsx

+ value: JoinRule.Knock,
+ label: _t("Ask to join"),
+ description: _t("Requires users to be granted access in order to join"),
+ checked: joinRule === JoinRule.Knock && knockingEnabled === true,


I think we might want to show this even if knockingEnabled === false to avoid lying to the user

This variable is to check whether knocking option is enabled in the labs flag or not, if not enabled will it be right to show this option?

Yes, though even if it is disabled, the room's join rule could be JoinRule.Knock - we should be smarter and show the user what the actual join rule is no matter what but we shouldn't allow the user to set the join rule to JoinRule.Knock if knockingEnabled === false

it's sort of expected that the labs flag will be moderately short-lived, so would be okay with it just being wrong when people don't have the labs flag enabled.

SimonBrandner · 2022-08-01T07:08:56Z

src/components/views/settings/JoinRuleSettings.tsx

@@ -49,13 +50,20 @@ interface IProps {
 const JoinRuleSettings = ({ room, promptUpgrade, aliasWarning, onError, beforeChange, closeSettingsFn }: IProps) => {
 const cli = room.client;

+ const roomSupportsKnocking = doesRoomVersionSupport(room.getVersion(), PreferredRoomVersions.KnockingRooms);
+ const preferredKnockingVersion = !roomSupportsKnocking && promptUpgrade


Is this unused?

Yeah going to use it to check the room version supports knocking or not

turt2live

generally this seems to be going in the correct direction!

For this PR, I think we'll just need the cypress tests and room version check mentioned elsewhere. Then we can get it landed and in front of users while we work on the rest of the features.

src/components/views/dialogs/CreateRoomDialog.tsx

turt2live · 2022-08-03T19:48:28Z

src/components/views/settings/JoinRuleSettings.tsx

+ value: JoinRule.Knock,
+ label: _t("Ask to join"),
+ description: _t("Requires users to be granted access in order to join"),
+ checked: joinRule === JoinRule.Knock && knockingEnabled === true,


it's sort of expected that the labs flag will be moderately short-lived, so would be okay with it just being wrong when people don't have the labs flag enabled.

ankur12-1610 · 2022-08-08T15:06:27Z

@turt2live I've made the recent push and I've also uploaded the video of the updated code which conditionally renders the room settings option according to labs flag, can you verify it? 😅

turt2live

looks to be in the right direction :)

please try to avoid force-pushing (rebasing) after review has already been given - it makes things a bit harder to review. We don't mind merge commits when updating the branch because we'll squash them all down to 1 when we merge the PR.

ankur12-1610 · 2022-08-10T07:49:48Z

looks to be in the right direction :)

please try to avoid force-pushing (rebasing) after review has already been given - it makes things a bit harder to review. We don't mind merge commits when updating the branch because we'll squash them all down to 1 when we merge the PR.

Agh! sorry for it I just do it in order avoid one extra merge commit everytime I update my branch 😅 I'll keep it in mind

SimonBrandner · 2022-08-16T08:36:54Z

cypress/e2e/knocking/knocking.spec.ts

+ cy.get('[label="Name"]').type(name);
+ cy.get('[label="Topic (optional)"]').type(topic);


I think we tend to use aria-label for selecting usually but it probably doesn't matter much

SimonBrandner · 2022-08-16T08:40:11Z

cypress/e2e/knocking/knocking.spec.ts

+ cy.get('[label="Topic (optional)"]').type(topic);
+ cy.get(".mx_JoinRuleDropdown").click();
+ cy.get(".mx_JoinRuleDropdown_knock").click();
+ cy.startMeasuring("from-submit-to-room");


Not sure what is the purpose of this? I am not sure if it makes sense to have a performance test specific to knocking?

Not sure what is the purpose of this? I am not sure if it makes sense to have a performance test specific to knocking?

I thought since we are using it in createRoom.spec.ts it has some importance or something like that

SimonBrandner · 2022-08-16T08:41:14Z

cypress/e2e/knocking/knocking.spec.ts

+
+ // Change room settings
+ cy.openRoomSettings("Security & Privacy");
+ cy.get(".mx_StyledRadioButton_content").contains("Ask to join").click();


/me might be confused - are you setting the join rule to the same thing again?

/me might be confused - are you setting the join rule to the same thing again?

Agh! thanks for it actually it was a bug in the JoinRuleSettings itself there wasn't an event message for join rule knock, I solved it now there shouldn't be a problem ;)

SimonBrandner · 2022-08-16T08:42:10Z

cypress/e2e/knocking/knocking.spec.ts

+ //Check if the room settings are visible if labs flag is disabled
+ cy.openUserSettings("Labs").within(() => {
+ //disables labs flag feature
+ cy.get("[aria-label='Knocking']").click();
+ });


If you could add disableLabsFeature analogous to enableLabsFeature, it would be absolutely awesome!

If you could add disableLabsFeature analogous to enableLabsFeature, it would be absolutely awesome!

I tried it but Idk why it isn't working could you please verify my latest pull 😅

Signed-off-by: ankur12-1610 <[email protected]>

…eature Signed-off-by: ankur12-1610 <[email protected]>

Signed-off-by: ankur12-1610 <[email protected]>

t3chguy · 2023-08-04T13:58:58Z

This has been superseded by PRs from Nordeck

ankur12-1610 requested a review from a team as a code owner July 23, 2022 20:35

ankur12-1610 requested review from kerryarchibald, duxovni, SimonBrandner and turt2live July 23, 2022 20:35

ankur12-1610 marked this pull request as draft July 23, 2022 20:36

ankur12-1610 mentioned this pull request Jul 23, 2022

WIP: Implement 'Knocking` element-hq/element-web#22925

Closed

3 tasks

github-actions bot deployed to Netlify July 23, 2022 20:40 View deployment

SimonBrandner reviewed Jul 24, 2022

View reviewed changes

ankur12-1610 force-pushed the knocking branch from d695cc9 to 738cda4 Compare July 24, 2022 10:55

github-actions bot deployed to Netlify July 24, 2022 10:59 View deployment

SimonBrandner reviewed Jul 24, 2022

View reviewed changes

ankur12-1610 mentioned this pull request Jul 24, 2022

WIP: Implement 'Knocking` element-hq/element-web#22926

Draft

3 tasks

turt2live removed request for kerryarchibald and duxovni July 29, 2022 21:46

ankur12-1610 force-pushed the knocking branch from 738cda4 to 9054db8 Compare August 1, 2022 07:04

github-actions bot deployed to Netlify August 1, 2022 07:08 View deployment

SimonBrandner reviewed Aug 1, 2022

View reviewed changes

turt2live reviewed Aug 3, 2022

View reviewed changes

github-actions bot deployed to Netlify August 4, 2022 05:53 View deployment

ankur12-1610 force-pushed the knocking branch 3 times, most recently from e1ed252 to 23a0c0f Compare August 8, 2022 13:29

github-actions bot deployed to Netlify August 8, 2022 13:33 View deployment

ankur12-1610 force-pushed the knocking branch from 190b949 to 3cd3f6b Compare August 8, 2022 15:04

github-actions bot deployed to Netlify August 8, 2022 15:09 View deployment

turt2live self-requested a review August 8, 2022 19:04

turt2live reviewed Aug 8, 2022

View reviewed changes

github-actions bot deployed to Netlify August 10, 2022 07:51 View deployment

github-actions bot deployed to Netlify August 16, 2022 08:25 View deployment

SimonBrandner reviewed Aug 16, 2022

View reviewed changes

ankur12-1610 force-pushed the knocking branch from fbef6d3 to 82bf75f Compare August 16, 2022 11:23

github-actions bot deployed to Netlify August 16, 2022 11:34 View deployment

ankur12-1610 force-pushed the knocking branch from 82bf75f to fbdf224 Compare August 16, 2022 15:00

github-actions bot deployed to Netlify August 16, 2022 15:04 View deployment

ankur12-1610 force-pushed the knocking branch from fbdf224 to 2651aab Compare August 16, 2022 15:13

github-actions bot deployed to Netlify August 16, 2022 15:18 View deployment

ankur12-1610 added 8 commits August 28, 2022 15:37

Added feature_knocking in Settings.tsx

f3ce267

Signed-off-by: ankur12-1610 <[email protected]>

Added knocking option in CreateRoomDialog

04af2d8

Signed-off-by: ankur12-1610 <[email protected]>

Added conditional rendering for the JoinRule DropDown based on labs f…

a4984b9

…eature Signed-off-by: ankur12-1610 <[email protected]>

Resolved the nits and added the knocking svg

942db0c

Signed-off-by: ankur12-1610 <[email protected]>

Added Ask to join option in Room Settings

aaecd18

Signed-off-by: ankur12-1610 <[email protected]>

Added iterative method to splice the definitions

23e9f63

Added cypress test for the implemented milestone

82993af

Resolved linting errors

8e7b212

ankur12-1610 force-pushed the knocking branch from 2651aab to 8e7b212 Compare August 28, 2022 10:07

github-actions bot deployed to Netlify August 28, 2022 10:11 View deployment

turt2live self-requested a review August 28, 2022 20:08

turt2live mentioned this pull request May 31, 2023

Implement Knocking on Element Web element-hq/element-web#18655

Open

turt2live removed their request for review June 6, 2023 19:49

t3chguy closed this Aug 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Implement `Knocking` #9093

WIP: Implement `Knocking` #9093

ankur12-1610 commented Jul 23, 2022 •

edited by github-actions bot

Loading

SimonBrandner left a comment

ankur12-1610 commented Jul 24, 2022

SimonBrandner left a comment

SimonBrandner left a comment

SimonBrandner Aug 1, 2022

ankur12-1610 Aug 1, 2022

SimonBrandner Aug 1, 2022

turt2live Aug 3, 2022

SimonBrandner Aug 1, 2022

ankur12-1610 Aug 1, 2022

turt2live left a comment

turt2live Aug 3, 2022

ankur12-1610 commented Aug 8, 2022 •

edited

Loading

turt2live left a comment

ankur12-1610 commented Aug 10, 2022

SimonBrandner Aug 16, 2022

SimonBrandner Aug 16, 2022

ankur12-1610 Aug 16, 2022

SimonBrandner Aug 16, 2022

ankur12-1610 Aug 16, 2022

SimonBrandner Aug 16, 2022

ankur12-1610 Aug 16, 2022

t3chguy commented Aug 4, 2023

		cy.get('[label="Name"]').type(name);
		cy.get('[label="Topic (optional)"]').type(topic);

WIP: Implement Knocking #9093

WIP: Implement Knocking #9093

Conversation

ankur12-1610 commented Jul 23, 2022 • edited by github-actions bot Loading

Checklist

SimonBrandner left a comment

Choose a reason for hiding this comment

ankur12-1610 commented Jul 24, 2022

SimonBrandner left a comment

Choose a reason for hiding this comment

SimonBrandner 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

turt2live left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ankur12-1610 commented Aug 8, 2022 • edited Loading

turt2live left a comment

Choose a reason for hiding this comment

ankur12-1610 commented Aug 10, 2022

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

t3chguy commented Aug 4, 2023

WIP: Implement `Knocking` #9093

WIP: Implement `Knocking` #9093

ankur12-1610 commented Jul 23, 2022 •

edited by github-actions bot

Loading

ankur12-1610 commented Aug 8, 2022 •

edited

Loading