search

jenkins-infra / stats.jenkins.io

Revamped Jenkins Infra Stats Website as a part of GSoC 2024
https://new.stats.jenkins.io
3 stars 4 forks source link

fix: fix a chart and stats table height issue from previous (PR #39) #57

Closed shlomomdahan closed 2 days ago

shlomomdahan commented 3 days ago

This PR makes sure that the size of the container holding the charts and table takes up the full size of the container, and well as ensuring that the mobile swappable drawer does not overlap the content when user is in mobile view.

shlomomdahan commented 2 days ago

@krisstern @Vandit1604

Can we merge if this looks OK?

Thanks

krisstern commented 2 days ago

This PR will need to be reviewed first before we could approve it and merge it.

@Vandit1604 could you please give this PR a review as soon as possible? Thanks!

shlomomdahan commented 2 days ago

@krisstern

I will open a separate PR for that.

That needs to be addressed on every single chart and I do not want to commingle the fixes on this PR as this one is specifically created to fix the height issue.

krisstern commented 2 days ago

@shlomomdahan If that is the case, please open an issue to track. Thanks!

shlomomdahan commented 2 days ago

@krisstern Issue #59 created to track chart titles

Vandit1604 commented 2 days ago

@shlomomdahan Can you resolve the merge conflicts before we merge this PR.

shlomomdahan commented 2 days ago

@krisstern @Vandit1604

Merge conflicts resolved

krisstern commented 2 days ago

@shlomomdahan There are some build failures in your PR, please fix before we can approve it

krisstern commented 2 days ago

BTW, can we add a feature to make the arrows clickable as well maybe in a new PR?

krisstern commented 2 days ago

Some are clickable and some are not currently, which is pretty awkward...

shlomomdahan commented 2 days ago

@krisstern Can you explain what you mean by clickable arrows? If you are referring to the mobile bottom drawer open arrows, then that is actually the intended functionality from MUIs component as it's meant to be for touchscreen devices. But I believe I can add the ability to have I open on clicking as well if that's preferable. However it might interfere with the swiping gesture.

Up to you guys. Open to adding it.

krisstern commented 2 days ago

@gounthar @Vandit1604 Which behavior do you guys prefer? And should we be consistent with it - i.e. either all arrows should be swipable or all should be clickable?