[13939] Implement system message into quo2 components #14061
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (45)
|
erikseppanen
requested review from
siddarthkay,
J-Son89,
flexsurfer,
Parveshdhull and
yqrashawn
Is ready for review or still WIP? |
briansztamfater
requested changes
Sep 22, 2022
flexsurfer
reviewed
Sep 23, 2022
J-Son89
suggested changes
Sep 23, 2022
There was a problem hiding this comment.
Found a few issues with the design implementation.
1 - pin icon seems too small.. in any case what seems to matter there is that the bounding box in the design is correct. Although if that's not there maybe it needs to be addressed with the avatar component that's used.
2 - the padding/margin between the 'pinned a message' and timestamp is not right.
3 - padding/margin between pin icon, user avatar and sub message ("Hello....") is not right.
4 - Text color seems wrong for "Hello! This is an example of a ... seems like it should be black and not grey.
5 - The background color of the pin icon should have 5% opacity. Seems like the underlying component needs to adjusted to handle different opacity settings.
J-Son89
reviewed
Sep 23, 2022
|
The padding/margin looks wrong here in multiple places. Also the line height is off by a fraction with some of the text. (This seems a bit nit picky though as it's so minor)
|
|
same here the icon and text padding/margin seems incorrect and then the button seems like it does not have the correct spacing between the right hand side of the component. |
erikseppanen
force-pushed
the
issue-13939
branch
3 times, most recently
from
71b81fb
to
7c676e3
Compare
erikseppanen
commented
Sep 27, 2022
yqrashawn
reviewed
Sep 27, 2022
yqrashawn
reviewed
Sep 27, 2022
J-Son89
approved these changes
Sep 27, 2022
briansztamfater
approved these changes
Sep 27, 2022
erikseppanen
force-pushed
the
issue-13939
branch
from
3e0bbf3
to
c24e027
Compare
erikseppanen
force-pushed
the
issue-13939
branch
from
c24e027
to
12f4a9a
Compare
erikseppanen
force-pushed
the
issue-13939
branch
8 times, most recently
from
a97d2f3
to
0536c47
Compare
siddarthkay
approved these changes
Oct 6, 2022
erikseppanen
force-pushed
the
issue-13939
branch
from
0536c47
to
52de86d
Compare
ibrkhalil
pushed a commit
that referenced
this pull request
Oct 20, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #13939
status: ready