matrix-react-sdk
matrix-react-sdk copied to clipboard
Allow user to set timezone
Hey,
This adds the ability for a user to set a Timezone which will replace the default one from the browser.
Worked on it mainly because Firefox with resist fingerprinting (Cf https://github.com/element-hq/element-web/issues/25006) set the timezone to UTC, and I'm not aware of an easy way to whitelist only specific domains.
Edit: probably should have searched more before coding but there is supposed to be a whitelist in Firefox RFP but for now I have issues with it :sweat_smile:. And this might be useful in other cases.
Checklist
- [ ] Tests written for new code (and old code if feasible).
- [ ] New or updated
public/exportedsymbols have accurate TSDoc documentation. - [x] Linter and other CI checks pass.
- [x] Sign-off given on the changes (see CONTRIBUTING.md).
Sorry wrong button
Not sure of the process but the PR is in a relative good state and working well.
Set it as WIP since:
- I'm aware of a test failing since the properties screen now has more element.
- Might need more translation ?
But wanted some feedback before spending more time on it :).
I've requested review from @matrix-org/product but they are quite busy so it may take some time, it is their decision what features land
I'd like @americanrefugee 's input on the design of the button and it's layout. In terms of functionality I would accept this enhancement if/when implemented correctly
Here's how I see it in the Netlify build. Let me know if I'm actually looking at the wrong thing.
Made a short recording of the field in action:
@americanrefugee I don't have access even with an account to the Figma file.
Sorry @Timshel I didn't realize at first that you're not an Element employee - which is why I can't give you access to our design files.
However, here are screenshots of how the feature should look like, as well as dropdown menu states:
As for the functionality, I agree that it's useful to be able to type to filter dropdown menu results. However, the user shouldn't actually be typing anything into the field.
Such a Dropdown component is being worked on in https://github.com/element-hq/compound-web/pull/204 and should be reusable here once it lands
Ok I'll switch to having the dropdown justified on the left with the label on top.
I agree that it's useful to be able to type to filter dropdown menu results. However, the user shouldn't actually be typing anything into the field.
I'll mention first that it's reusing the same dropdown that the one used at the top of the page for Application language.
But I'm not sure what you mean by not "typing anything into the field" do you mean not visible ? The filtering is essentially a search, without the "search" term visible the user won't be able to see if he made a typo, and it will make deleting the search term quite tricky.
If you mean it to work like the classic list with keyboard navigation then I would mention that there is over 400 values so I believe filtering is quite important here.
I suppose if the user starts typing then it can follow the regular form field component:
And here is the Clear / X button:
- By default, the selected menu item label should appear
- If typing, the content are replaced with a cursor and "Clear" (X) button
- The list should update as the user types
- If the user clicks the x / clear button, the original label re-appears, the up arrow reappears, and all menu items are shown
Something like that?
@americanrefugee adding the Clear/ X would make sense, but I believe there is some "conflict" with the down arrow which indicate that it's a dropdown (probably why it's not already present).
As I mentioned I'm just reusing an existing component, so any change would impact multiple existing fields. To be honest I believe this is a separate issue and should not be addressed here; and since this handling is already used in the same page I don't believe it should block this PR.
Ok, reuse the existing component. Don't wanna block anything :)
@americanrefugee do you have a recommendation for the dropdown width ?
By default, it matches the width of the selected option (The container has the class mx_SettingsSubsection_contentStretch).
For the other dropdown (Application dropdown ) it matches the hint width (The app will reload after selecting another language).
Added the current Timezone in the Browser default placeholder/option using the Timezone short format defined as:
Short localized form (e.g.: PST, GMT-8)
Cf https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/DateTimeFormat
For now it look like this
do you have a recommendation for the dropdown width ?
360 pixels
@americanrefugee page look look like snapshots/settings/preferences-user-settings-tab.spec.ts
Set the 360px using min-width to prevent wrapping if the application language description is wider.
Hey, I suppose someone need to review the code ? :)
@daniellekirkwood given @americanrefugee is unavailable how can we progress on this?
Any news ?
Any news ?
LGTM. Is there anything else do you need from me atm?
Any news ?
LGTM. Is there anything else do you need from me atm?
Can you approve the PR ? Thanks :)
@Timshel Can you merge develop in the PR in order to solve the conflict and make it up to date ? I'll review it after. Thanks!
I rebased but the Playwright tests should fail since I'm unable to run those (and update the snapshot); it fails with :
Test timeout of 30000ms exceeded while setting up "user".
I tested on develop too and had the same error.
I rebased but the Playwright tests should fail since I'm unable to run those (and update the snapshot); it fails with :
Test timeout of 30000ms exceeded while setting up "user".I tested on
developtoo and had the same error.
Why are you unable to run the playwright tests?
It's failing in:
at ../element-web-test.ts:230
228 | user: async ({ pageWithCredentials: page, credentials }, use) => {
229 | await page.goto("/");
> 230 | await page.waitForSelector(".mx_MatrixChat", { timeout: 30000 });
Raised the timeout to 60000 but same error.
Edit: I'm running just the preference test with
yarn run test:playwright playwright/e2e/settings/preferences-user-settings-tab.spec.ts
I think you have this error because the local instance of synapse used by the tests is not here. Normally, it's launched automatically when you launch our playwright tests.
Can you reinstall playwright and also put all the output of yarn run test:playwright playwright/e2e/settings/preferences-user-settings-tab.spec.ts
The synapse instance appears to be running, the output from the run : https://gist.github.com/Timshel/a4ee0cdac04f7ded5fb4098cb4f74597 I had a quick look and homeserver-stdout.log / homeserver-stderr.log does not appear to contain any errors.
I wiped the node_modules and use yarn cache clean before installing again but no change.
Edit: and I pulled the latest image with docker pull ghcr.io/element-hq/synapse:develop.
Do you have an ED/EW running when you launch the playwright tests?
- If no => start it
- If yes => stop it
(seems weird but trust me :p)
Not sure what you mean by:
- ed: editor ?
- ew: element web ?
Kept only a shell open to run the tests and no change.
Unless you want to debug the issue, might be easier, if you don't mind, if you commit the updated snapshot ? (you should have commit right to the feature branch of the PR).
Sorry to ask it but for now I'm not working on any other modification so I would prefer not to spend the time to dig the issue (I'm using Playwright in another project which control some docker container without any issue).
Edit: ED: guessing it meant Element Desktop ?
Ho sorry, EW => Element Web. I'll update the snapshots then after the review
I had missed the errors from Typescript Syntax Check fixing those now.