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

feat:show table in mobile with hscroll #1273

Merged
merged 15 commits into from
Aug 22, 2024

Conversation

rajeshj11
Copy link
Contributor

@rajeshj11 rajeshj11 commented Aug 21, 2024

/closes #969
showing the table and providing the horizontal scroll if any overflow is there.

Impact: mobile device will able to view the table data.

@DonKoko
Copy link
Contributor

DonKoko commented Aug 21, 2024

@carlosvirreira Rajesh has taken on this issue. He has updated all tables in the platform to make them horizontally scrollable so users can see full data.
I am deploying it to staging: https://github.com/Shelf-nu/shelf.nu/actions/runs/10490727074

Lets do some testing to make sure everything is okey.

Copy link
Contributor

@DonKoko DonKoko left a comment

Choose a reason for hiding this comment

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

Writing some feedback, based on what we discussed:

  • Some <Td> components have className="table-cell". This does nothing as they are already table cells. Lets remove it
  • With the new approach the name column(first column) should not grow anymore(only mobile). It should just shrink to the content. Basically default table functionality. This is the case on some pages (assets) but not the case on the bookings index. Should be consistent. Also please check all indecies because in the categories index, for example, the description column grows. Again we dont want that on mobile, just desktop.
  • The gradient is a bit too thin(change to 35px). Also the gradient at the left point has to be 100% transparent so we have a smooth transition. Now it looks like a cut-off line
  • The gradient should only be there if the table is overflowing horizontally(both desktop and mobile). For example in the Non-registered members index, there are very few columns so they fit and there is no horizontal scroll, however there is still a gradient covering the right column
  • team/users index still has the chevrons, even tho it has the new style of table. Needs to be adjusted
  • Booking/manage-assets modal is basically broken now. I cant scroll it or view it. Same for location/manage-assets modal.

@carlosvirreira
Copy link
Contributor

carlosvirreira commented Aug 21, 2024

  • Batch Action is broken on Mobile (Safari) - On Asset Index it works well

Perhaps adjacently related:

On asset dashboard - some widgets seem to assume they are side-scroll-able and therefore show some sort of shadow on the right side.

Same as locations on mobile. Action panel looks broken.

Calendar component also seems to be cracking.

User table on mobile seems to also show the right pointing chevron to show its click-able but also sidescroll.

@DonKoko my comments.

IMG_7759

@rajeshj11
Copy link
Contributor Author

rajeshj11 commented Aug 21, 2024

Writing some feedback, based on what we discussed:

  • Some <Td> components have className="table-cell". This does nothing as they are already table cells. Lets remove it
  • With the new approach the name column(first column) should not grow anymore(only mobile). It should just shrink to the content. Basically default table functionality. This is the case on some pages (assets) but not the case on the bookings index. Should be consistent. Also please check all indecies because in the categories index, for example, the description column grows. Again we dont want that on mobile, just desktop.
  • The gradient is a bit too thin(change to 35px). Also the gradient at the left point has to be 100% transparent so we have a smooth transition. Now it looks like a cut-off line
  • The gradient should only be there if the table is overflowing horizontally(both desktop and mobile). For example in the Non-registered members index, there are very few columns so they fit and there is no horizontal scroll, however there is still a gradient covering the right column
  • team/users index still has the chevrons, even tho it has the new style of table. Needs to be adjusted
  • Booking/manage-assets modal is basically broken now. I cant scroll it or view it. Same for location/manage-assets modal.

resolved all the issues. I am not 100% sure about the gradient(CSS background thing. i might need help on that).

@rajeshj11
Copy link
Contributor Author

  • Batch Action is broken on Mobile (Safari) - On Asset Index it works well

Perhaps adjacently related:

On asset dashboard - some widgets seem to assume they are side-scroll-able and therefore show some sort of shadow on the right side.

Same as locations on mobile. Action panel looks broken.

Calendar component also seems to be cracking.

User table on mobile seems to also show the right pointing chevron to show its click-able but also sidescroll.

@DonKoko my comments.

IMG_7759

I am not sure about the calendar thing. @DonKoko I have checked with main branch it is same. other issues were taken care.

@rajeshj11 rajeshj11 requested a review from DonKoko August 22, 2024 05:29
Copy link
Contributor

@DonKoko DonKoko left a comment

Choose a reason for hiding this comment

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

  • with the new approach, the description column on categories index should not be truncated in 1 row. Can you make it that it takes 3 rows. We have a function alreayd in shelf that counts the rows and truncates the text

For the gradient do the following:

  1. Increase width to 35px
  2. Set the gradient to : linear-gradient(to right, rgba(255, 255, 255, 0) 0%, #fff 100%)

@rajeshj11
Copy link
Contributor Author

  • with the new approach, the description column on categories index should not be truncated in 1 row. Can you make it that it takes 3 rows. We have a function alreayd in shelf that counts the rows and truncates the text

For the gradient do the following:

  1. Increase width to 35px
  2. Set the gradient to : linear-gradient(to right, rgba(255, 255, 255, 0) 0%, #fff 100%)

ack

@carlosvirreira
Copy link
Contributor

What I meant about the calendar.

IMG_7762

Besides that, looks good to me (safari, iPhone)

};

checkOverflow(); // Initial check
window.addEventListener("resize", checkOverflow); // Check on resize
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to do this, inside the useEffect you should have a if (window)

const containerRef = useRef<HTMLDivElement>(null);
const [isOverflowing, setIsOverflowing] = useState(false);

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please abstract this into its own hook called useTableIsOverflowing which returns the value?

@DonKoko DonKoko merged commit 51231e1 into Shelf-nu:main Aug 22, 2024
4 checks passed
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.

[Feature request]: Show who created a booking on the booking page
3 participants