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.

Sign up for GitHub

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

Jump to bottom

The margin to the right of the avatar in a chat message should be 12px #14006

Closed
6 tasks
kavimuru opened this issue Jan 5, 2023 · 31 comments
Closed
6 tasks

The margin to the right of the avatar in a chat message should be 12px #14006

kavimuru opened this issue Jan 5, 2023 · 31 comments
Assignees
@shawnborton @sobitneupane @grgia @dylanexpensify
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@kavimuru
Copy link

kavimuru commented Jan 5, 2023 •
edited by melvin-bot bot
Loading

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Action Performed:

  1. Open Newdot
  2. Open any chat
  3. Observe the margin right to the avatar

Expected Result:

Should be 12px like in chat header and LHN

Actual Result:

It is 8px

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.2.48-2
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
image (1)
Untitled

Expensify/Expensify Issue URL:
Issue reported by: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1672827156356519

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~014f91d608cd8fd8d0
  • Upwork Job ID: 1612400931481505792
  • Last Price Increase: 2023-01-09
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 5, 2023

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot locked and limited conversation to collaborators Jan 5, 2023
@shawnborton shawnborton self-assigned this Jan 5, 2023
@grgia grgia self-assigned this Jan 5, 2023
@grgia
Copy link
Contributor

grgia commented Jan 5, 2023

I'm happy to take this one!

@melvin-bot melvin-bot bot added the Overdue label Jan 9, 2023
@dylanexpensify
Copy link
Contributor

reviewing today!

@melvin-bot melvin-bot bot removed the Overdue label Jan 9, 2023
@dylanexpensify
Copy link
Contributor

Nice! @grgia going to mark internal then since you got this!

@dylanexpensify dylanexpensify added the Internal Requires API changes or must be handled by Expensify staff label Jan 9, 2023
@melvin-bot melvin-bot bot unlocked this conversation Jan 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 9, 2023

Job added to Upwork: https://www.upwork.com/jobs/~014f91d608cd8fd8d0

@melvin-bot
Copy link

melvin-bot bot commented Jan 9, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @sobitneupane (Internal)

@grgia
Copy link
Contributor

grgia commented Jan 9, 2023

cc @shawnborton - we were fixing some alignments in this PR that I'm wrapping up today, is this issue a design choice or a bug I should add to that PR?

@shawnborton
Copy link
Contributor

Ah good reminder. Yeah, I think we want to make sure the compose bar item alignment matches this 12px right padding. It looks like in my mockups, I had been working under the assumption that the margin to the right of chat avatars was 12px:
image

@grgia grgia mentioned this issue Jan 9, 2023
Merged
60 tasks
@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Jan 9, 2023
@grgia
Copy link
Contributor

grgia commented Jan 9, 2023

Quick follow up question,
Right now on staging it looks like its a 16px gap on LHN/Header, In figma, it's 12px. Should I update to 12px everywhere? Or just update the chat items to 16px?

image

@grgia
Copy link
Contributor

grgia commented Jan 9, 2023

Here's with 16px for reference:
image

@shawnborton
Copy link
Contributor

Ah, let's fix that header margin to be 12px as well.

@dylanexpensify
Copy link
Contributor

@grgia any update here? 🙇‍♂️

@shawnborton
Copy link
Contributor

Just merged this one!

@dylanexpensify
Copy link
Contributor

NICE! Thanks @shawnborton !

@melvin-bot
Copy link

melvin-bot bot commented Jan 20, 2023

@shawnborton, @sobitneupane, @grgia, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@sobitneupane
Copy link
Contributor

Payment is due for C+ review.

cc: @dylanexpensify

@dylanexpensify
Copy link
Contributor

On it!

@dylanexpensify
Copy link
Contributor

@sobitneupane sent invite to apply for job!

@MelvinBot
Copy link

@shawnborton, @sobitneupane, @grgia, @dylanexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@dylanexpensify
Copy link
Contributor

@sobitneupane payment sent!

@dylanexpensify
Copy link
Contributor

For Regression Test step @shawnborton do we think this is something we should add a step for? I wonder if it might be something captured by visual guidelines, but curious for your thoughts!

@MelvinBot
Copy link

@shawnborton, @sobitneupane, @grgia, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@dylanexpensify
Copy link
Contributor

just bumping the above comment @shawnborton! 🙇‍♂️

@shawnborton
Copy link
Contributor

Good question. I suppose we could add a step to make sure that there is always 12px of space between the avatar and the sender name? But yeah, maybe that would be captured by the visual guidelines whenever those are ready to go?

@dylanexpensify
Copy link
Contributor

Yeah, lemme cc @mallenexpensify to see his thoughts! I think this might be more a visual guideline bit than a test step IMO

@mallenexpensify
Copy link
Contributor

I added to the Testing guidelines doc under design - link

We shouldn't need to create or update anything in TestRail.

@MelvinBot
Copy link

@shawnborton, @sobitneupane, @grgia, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@MelvinBot
Copy link

@shawnborton, @sobitneupane, @grgia, @dylanexpensify Huh... This is 4 days overdue. Who can take care of this?

@MelvinBot
Copy link

@shawnborton, @sobitneupane, @grgia, @dylanexpensify Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

@MelvinBot
Copy link

@shawnborton, @sobitneupane, @grgia, @dylanexpensify 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

@mallenexpensify
Copy link
Contributor

Pretty sure this can be closed as regression test steps were added to the testing guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shawnborton shawnborton

@sobitneupane sobitneupane

@grgia grgia

@dylanexpensify dylanexpensify

Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@shawnborton @MelvinBot @mallenexpensify @sobitneupane @grgia @kavimuru @dylanexpensify