Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement typing indicator theme-v2 #95 #130

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

szuperaz
Copy link
Collaborator

@szuperaz szuperaz commented Jun 21, 2022

🎯 Goal

Describe why we are making this change

🛠 Implementation details

Provide a description of the implementation

🎨 UI Changes

GetStream/stream-chat-angular#315

closes #95

@szuperaz szuperaz marked this pull request as ready for review June 21, 2022 08:22
| `--str-chat__typing-indicator-border-block-start` | Top border of the component |
| `--str-chat__typing-indicator-border-block-end` | Bottom border of the component |
| `--str-chat__typing-indicator-border-inline-start` | Left (right in RTL layout) border of the component |
| `--str-chat__typing-indicator-border-inline-start` | Right (lrft in RTL layout) border of the component |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| `--str-chat__typing-indicator-border-inline-start` | Right (lrft in RTL layout) border of the component |
| `--str-chat__typing-indicator-border-inline-end` | Right (left in RTL layout) border of the component |

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, that is a copy-paste mistake copied a lot of times 😀, will take care of this everywhere

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will fix this after the ongoing PRs merged into theming-v2

--str-chat__typing-indicator-border-inline-start: none;
--str-chat__typing-indicator-border-inline-end: none;
--str-chat__typing-indicator-box-shadow: none;
--str-chat__typing-indicator-dot-background-color: var(--str-chat__text-color);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
--str-chat__typing-indicator-dot-background-color: var(--str-chat__text-color);
--str-chat__typing-indicator__dot-background-color: var(--str-chat__text-color);

Nitpick: I'm not sure how closely BEM is followed across other files but, I think dot represents an element. This just caught my eye :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In theme-v2 for CSS variables we use camel-case after the str-chat__ prefix

Copy link
Contributor

Choose a reason for hiding this comment

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

I think BEM is just for class names, but I'm happy to be corrected. Let's stick to snake-case for CSS variables.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, our CSS class names are not 100% consistent, it's mostly BEM, but there are exceptions, the only consistent part is the str-chat__ prefix.

Copy link
Collaborator Author

@szuperaz szuperaz Jun 22, 2022

Choose a reason for hiding this comment

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

#135 created an issue to create some lint rules to check class and variable names

@szuperaz szuperaz merged commit 352f971 into theming2 Jun 22, 2022
@szuperaz szuperaz deleted the typing-indicator-theme-v2 branch June 22, 2022 08:02
@szuperaz
Copy link
Collaborator Author

🎉 This PR is included in version 3.0.0-theming2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@szuperaz
Copy link
Collaborator Author

szuperaz commented Sep 9, 2022

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants