Skip to content

Fixed alignment of typing indicator in chat composite by changing flexFlow to column-reverse#1297

Merged
prabhjot-msft merged 6 commits intomainfrom
prabhjot/bug-2662655
Jan 13, 2022
Merged

Fixed alignment of typing indicator in chat composite by changing flexFlow to column-reverse#1297
prabhjot-msft merged 6 commits intomainfrom
prabhjot/bug-2662655

Conversation

@prabhjot-msft
Copy link
Copy Markdown
Contributor

@prabhjot-msft prabhjot-msft commented Jan 11, 2022

What

Fixed alignment of typing indicator in chat composite by changing flexFlow to column-reverse

Why

https://skype.visualstudio.com/SPOOL/_workitems/edit/2662655

How Tested

Manually

Process & policy checklist

  • I have updated the project documentation to reflect my changes if necessary.
  • I have read the CONTRIBUTING documentation.

Is this a breaking change?

  • This change causes current functionality to break.

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" label to the PR for updating the snapshot.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "ui change" label to the PR for updating the snapshot.

@github-actions
Copy link
Copy Markdown
Contributor

*/
export const typingIndicatorContainerStyle = mergeStyles({
minHeight: '2.125rem'
minHeight: '0.125rem'
Copy link
Copy Markdown
Member

@JamesBurnside JamesBurnside Jan 12, 2022

Choose a reason for hiding this comment

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

The min-height is actually there to prevent the messages from shifting up and down when a typing indicator appears (this css line should have a comment to make that clear).

e.g. without the minHeight, when a typing indicator appears and disappears it causes the messages to shift up and down:
image

Is it possible to keep the minHeight and instead align the text (vertically) to the bottom or center of the container so it is not so far away from the edit box?

Copy link
Copy Markdown
Member

@JamesBurnside JamesBurnside left a comment

Choose a reason for hiding this comment

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

Want to check if we can keep the min height or not

@prabhjot-msft prabhjot-msft changed the title Fixed alignment of typing indicator in chat composite by reducing minHeight Fixed alignment of typing indicator in chat composite by changing flexFlow to column-reverse Jan 12, 2022
@prabhjot-msft
Copy link
Copy Markdown
Contributor Author

Want to check if we can keep the min height or not

We can keep the min height and align the text to bottom by changing flexFlow to column-reverse, let me know if that works.

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown
Contributor

minHeight: '2.125rem'
minHeight: '2.125rem',
// flexFlow set to column-reverse to align the text to the bottom of the container
flexFlow: 'column-reverse'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

because flex boxes have a wide variety of css properties that are hard to dissociate from each other (and because when changing css its incredibly hard to ensure no other properties are affected) we want to avoid writing css as much as possible.

Instead we want to use a semantic driven approach and use Fluent's Stack
These have properties like horiztonal and align=end to avoid having to write css and have a semantical-driven approach to how components are laid out that very readable&understandable.

Instead can you do something like:
image
? (not sure if the alignment should be 'baseline' or 'end')

If not keep this as is!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

flex layout is taking precedence over the properties thus vertical align is not working in this case. The other property that we can change is justifyContent='end' which is again property to flex layout. So, I am going to use column-reverse in this case.

Copy link
Copy Markdown
Member

@JamesBurnside JamesBurnside left a comment

Choose a reason for hiding this comment

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

Just a suggestion on avoid css, otherwise this is great!

@github-actions
Copy link
Copy Markdown
Contributor

@prabhjot-msft prabhjot-msft merged commit 65d0ae5 into main Jan 13, 2022
@prabhjot-msft prabhjot-msft deleted the prabhjot/bug-2662655 branch January 13, 2022 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants