Skip to content

Conversation

@SupianIDz
Copy link

Hi, thank you for creating this cool template, but I think it would be better if the height of the space usage chart was increased a little. Because it is too close to the username.

BEFORE
before

AFTER
after

Thanks

Copy link
Owner

@thexdev thexdev left a comment

Choose a reason for hiding this comment

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

Hi, I've reviewed your commit change. Please complete the request changes and I'll be ready to merge your commit change :)

const SpaceUsage = () => {
return (
<div className="flex flex-wrap items-end content-end justify-center mt-6 relative h-32">
<div className="flex flex-wrap items-end content-end justify-center mt-6 relative h-40">
Copy link
Owner

Choose a reason for hiding this comment

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

You don't need to change the height. It will break the UI when the screen resolution is lower than 1920 x 1080.

Suggested change
<div className="flex flex-wrap items-end content-end justify-center mt-6 relative h-40">
<div className="flex flex-wrap items-end content-end justify-center mt-6 relative h-32">

return (
<div className="flex flex-wrap items-end content-end justify-center mt-6 relative h-32">
<div className="flex flex-wrap items-end content-end justify-center mt-6 relative h-40">
<img src={Chart} alt="Space usage" className="absolute w-full" />
Copy link
Owner

Choose a reason for hiding this comment

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

Just add class h-full after w-full and the image will perfectly fit on every screen.

Suggested change
<img src={Chart} alt="Space usage" className="absolute w-full" />
<img src={Chart} alt="Space usage" className="absolute w-full h-full" />

purge: ['./src/*.js', './src/components/**/*.js'],
theme: {
extend: {},
height: {
Copy link
Owner

Choose a reason for hiding this comment

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

And now you no longer need to change the config. Just remove this commit and I'll be ready to merge your commit change :)

@thexdev
Copy link
Owner

thexdev commented Aug 9, 2020

Hi, thanks for the correction. It looks like something like this happens on screens that are equal to or larger than 1920 x 1080. This is caused by the image overflowing of the wrapper.

I've reviewed your commit change and I think you shouldn't need to change the height because this problem comes from the image not from the wrapper.

If you don't mind you can add a class h-full to the image instead of increasing the height of the wrapper so the image doesn't overflow from the wrapper and fit to every screen size.

And please use commit convention (semantic commit) so the commit changes in this repository look consistent. You can also use gitmoji to describe what you have done.

These links might helpful:

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.

2 participants