Make ToastContainer compatible with RTL layout #8230
Conversation
github-actions
bot
added
the
T-Task
Codecov Report
@@ Coverage Diff @@
## develop #8230 +/- ##
========================================
Coverage 29.82% 29.82%
========================================
Files 875 875
Lines 50024 50024
Branches 12727 12727
========================================
Hits 14918 14918
Misses 35106 35106
|
Signed-off-by: Suguru Hirahara <[email protected]>
- Use properties available for flexbox - Align the title of the toast to the center Signed-off-by: Suguru Hirahara <[email protected]>
Signed-off-by: Suguru Hirahara <[email protected]>
| @@ -108,12 +108,13 @@ limitations under the License. | |||
| } | |||
|
|
|||
| .mx_Toast_title { | |||
| display: flex; | |||
There was a problem hiding this comment.
I don't think we need a flexbox here? What is it fixing?
There was a problem hiding this comment.
- it aligns the title and the
iconcounter (if any) horizontally - it assures the gap between the title and the
iconcounter without usingmarginpadding-left: 8px; - it automatically calculates the flow of the child elements
I edited the first post to explain the reason why flexbox should realize what is intended here.
| @@ -122,12 +123,11 @@ limitations under the License. | |||
| vertical-align: middle; | |||
| } | |||
|
|
|||
| span { | |||
| padding-left: 8px; | |||
| float: right; | |||
There was a problem hiding this comment.
this might have been deliberate, even in a RTL language - will have to check with design.
There was a problem hiding this comment.
I don't think this is intended ↓
Unless this would be expected to cover something in addition to <span>{ countIndicator }</span>, this should work.
luixxiul
changed the title
|
@turt2live could you please review again? Basically this PR does not fix something what users face currently, instead it makes the toast compatible with RTL layout worked on #8217. |
Signed-off-by: Suguru Hirahara <[email protected]>
|
The improvements proposed in the PR description to make the toast work as expected in RTL mode make sense to me. 👍 I won't go into more details as this component is pending a proper, in-depth redesign. |
Signed-off-by: Suguru Hirahara [email protected]
Cherry-picked from #8217
This PR removes the unnecessary rules which have not worked.
I guess the rules have been forgotten to be removed.
This also replaces float + margin + line-height properties with ones available for flexbox.
Why flexbox?
Because it realizes what is intended with the current style rules.
At first, the intention of these rules is to align the title on the middle of the area.
Next, the counter is placed on the middle of the line by specifying the same line-height property as the title:
If you remove the line-height property manually, you should get the result as below.
By the way, one of the the intentions of flexbox property is:
Therefore, flexbox is expected to realize what is intended here.
Current implementation:
LTR layout:
RTL layout (emulating it by adding
dir="rtl"to thehtmltag):margin-leftdoes not create the gap between the buttonsWith this PR:
LTR layout:
RTL layout:
Yellow: margin
Purple: padding
These right side padding areas in the title and description areas should be taken care of by #8217.
type: task
This change is marked as an internal change (Task), so will not be included in the changelog.
Preview: https://pr8230--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.