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

RTL layout compatibility: bridges list on room settings page #8243

Merged
merged 22 commits into from
Apr 19, 2022
Merged

RTL layout compatibility: bridges list on room settings page #8243

merged 22 commits into from
Apr 19, 2022

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Apr 6, 2022

Addresses element-hq/element-web#14520 (#8217)

Current implementation:

LTR layout
before

RTL layout
before2

The icon is moved to the left side because of the float: right;. This PR fixes the bug by replacing float with flexbox (note: you could otherwise set float: left for the RTL layout by either replacing the current padding setting with a logical one and setting a specific className for the layout to a higher element).

after1
after2

The position of the avatar inside pill is out of scope of this PR. It should be taken care of by #8217.

Multiple bridges:

after3
after4

Signed-off-by: Suguru Hirahara [email protected]

type: task


This change is marked as an internal change (Task), so will not be included in the changelog.

Preview: https://pr8243--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

luixxiul added 11 commits April 6, 2022 11:19
- CSS logical properties
- Replace float with flexbox

Set column-gap:10px to keep 10px space between column-icon and column-data

Also: rename protocol-icon to protocolIcon

Signed-off-by: Suguru Hirahara <[email protected]>
Usage of wildcard should be minimised as it consumes resource more than
necessary

Signed-off-by: Suguru Hirahara <[email protected]>
Remove padding:0 and border:0 of a list inside another list with proper nesting

Signed-off-by: Suguru Hirahara <[email protected]>
Signed-off-by: Suguru Hirahara <[email protected]>
Signed-off-by: Suguru Hirahara <[email protected]>
Signed-off-by: Suguru Hirahara <[email protected]>
fix the initial letter of the bridge at the center of the rectangle

Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul luixxiul requested a review from a team as a code owner April 6, 2022 14:57
@codecov
Copy link

codecov bot commented Apr 6, 2022

Codecov Report

Merging #8243 (1ae79ee) into develop (6b13988) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff            @@
##           develop    #8243   +/-   ##
========================================
  Coverage    30.03%   30.03%           
========================================
  Files          881      881           
  Lines        50213    50213           
  Branches     12791    12791           
========================================
  Hits         15081    15081           
  Misses       35132    35132           
Impacted Files Coverage Δ
src/components/views/settings/BridgeTile.tsx 2.70% <0.00%> (ø)


span {
/* Correct letter placement */
left: auto;
Copy link
Contributor Author

@luixxiul luixxiul Apr 6, 2022

Choose a reason for hiding this comment

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

This might be no longer necessary thanks to box-sizing above, but I'll keep this for now.

@github-actions github-actions bot added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Apr 7, 2022
@luixxiul luixxiul marked this pull request as draft April 9, 2022 07:55
@luixxiul luixxiul marked this pull request as ready for review April 9, 2022 11:20
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

Looks sane otherwise

src/components/views/settings/BridgeTile.tsx Outdated Show resolved Hide resolved
src/components/views/settings/BridgeTile.tsx Outdated Show resolved Hide resolved
res/css/views/dialogs/_RoomSettingsDialogBridges.scss Outdated Show resolved Hide resolved
@luixxiul
Copy link
Contributor Author

Thank you very much for checking. I'm going to address them soon 😄

Signed-off-by: Suguru Hirahara <[email protected]>
Signed-off-by: Suguru Hirahara <[email protected]>
res/css/_common.scss Outdated Show resolved Hide resolved
@luixxiul luixxiul marked this pull request as draft April 13, 2022 17:15
Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul luixxiul marked this pull request as ready for review April 13, 2022 17:23
@luixxiul luixxiul requested a review from t3chguy April 13, 2022 18:11
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

Looks sane to me, thanks!

@t3chguy t3chguy enabled auto-merge (squash) April 19, 2022 09:05
@t3chguy t3chguy disabled auto-merge April 19, 2022 09:05
@t3chguy t3chguy enabled auto-merge (squash) April 19, 2022 09:06
@t3chguy
Copy link
Member

t3chguy commented Apr 19, 2022

Can you please pull latest develop

@luixxiul
Copy link
Contributor Author

Done, but it's stuck like this:

Screenshot_2022-04-19_11-14-37

I'm not really sure what that means...

@t3chguy t3chguy merged commit 80c1fad into matrix-org:develop Apr 19, 2022
@luixxiul luixxiul deleted the RoomSettingsDialogBridges branch April 19, 2022 11:41
Johennes pushed a commit to Johennes/matrix-react-sdk that referenced this pull request Apr 19, 2022
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.

2 participants