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

[FIX] Retry icon comes out of the div #20390

Merged
merged 8 commits into from
Feb 17, 2021
Merged

Conversation

im-adithya
Copy link
Contributor

@im-adithya im-adithya commented Jan 25, 2021

Proposed changes (including videos or screenshots)

Changed the height of the div container.

Issue(s)

This PR closes #20389

Earlier

Screenshot (41)

Now

Screenshot (44)

Note

Due to different image sizes, the widths are different here in the image but actually the boxes are of the same width and the retry icon and text are of the same size.

@gabriellsh
Copy link
Member

Instead of changing the sizes, wouldn't a simple padding in the placeholder suffice?

@gabriellsh
Copy link
Member

Please, also post screenshots of the fix. Thanks!

@im-adithya
Copy link
Contributor Author

Instead of changing the sizes, wouldn't a simple padding in the placeholder suffice

Sure! This seems to be better indeed!!
I'll implement this

@im-adithya im-adithya changed the title Update Image.tsx [FIX] Retry icon comes out of the div Jan 26, 2021
@lgtm-com
Copy link

lgtm-com bot commented Jan 26, 2021

This pull request introduces 16 alerts when merging fa495c6 into 54e0a71 - view on LGTM.com

new alerts:

  • 16 for Syntax error

@im-adithya
Copy link
Contributor Author

@dougfabris @ggazzo can anyone please review this? Thanks!

Copy link
Contributor

@tiagoevanp tiagoevanp left a comment

Choose a reason for hiding this comment

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

Other suggestion, remove the {...props} in that component, there is no necessity to pass dimensions for that.

client/components/Message/Attachments/components/Image.tsx Outdated Show resolved Hide resolved
tiagoevanp
tiagoevanp previously approved these changes Feb 13, 2021
Copy link
Contributor

@tiagoevanp tiagoevanp left a comment

Choose a reason for hiding this comment

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

The { ...dimensions } on Retry componente must be removed... anyways, approved.

@im-adithya
Copy link
Contributor Author

Thank you @tiagoevanp! Removed {...dimensions} as well!

@lgtm-com
Copy link

lgtm-com bot commented Feb 13, 2021

This pull request introduces 1 alert when merging 4728cc0 into 4f05c55 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@ggazzo ggazzo merged commit b3eb3e0 into RocketChat:develop Feb 17, 2021
@sampaiodiego sampaiodiego mentioned this pull request Feb 28, 2021
@im-adithya im-adithya deleted the retry-size branch April 27, 2021 13:29
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.

Retry icon comes out of the box
4 participants