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

Table column initial width setting #13610

Merged
merged 24 commits into from May 20, 2024
Merged

Table column initial width setting #13610

merged 24 commits into from May 20, 2024

Conversation

aptkingston
Copy link
Member

Description

This PR adds an initial width setting to columns in tables. We had a width setting with the old table, but we didn't add it for the new table because

  • You can drag to resize columns as you like
  • The widths default to those in the data section, so should already be roughly correct

There are still valid use cases for having unique widths per table instance, so this PR restores that width setting. This setting is an "initial width" setting rather than an explicit width - that is to say that users can still resize columns as they like, but this setting will change the initial widths to those specified by the app builder.

image

Launchcontrol

Add initial width setting to table columns.

@@ -100,9 +100,6 @@
on:click={() => {
get(store).actions.select(draggableItem.id)
}}
on:mousedown={() => {
get(store).actions.select()
}}
Copy link
Member Author

Choose a reason for hiding this comment

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

Not needed - this was just closing the previous popover on mouse down which isn't how we usually handle popovers so I'm removing this to keep things consistent with just clicks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Bit of a refactor of this file to ensure popovers don't accidentally close when component remounts. Using the open prop on the popover is better than binding to the component API and using the show and hide methods.

Also enables clicking on the icon to toggle open/close state, which currently doesn't work.

Copy link
Member Author

Choose a reason for hiding this comment

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

This custom position handler is no longer needed, as my updated logic for the usual position_dropdown util already handles this. I haven't specifically checked usage with the tours yet but I will.

Copy link
Contributor

@deanhannigan deanhannigan left a comment

Choose a reason for hiding this comment

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

Some minor issues but the changes are solid!

Bug - Context menu for the Table now floating to the top in the builder preview

@aptkingston
Copy link
Member Author

Some minor issues but the changes are solid!

Bug - Context menu for the Table now floating to the top in the builder preview

Fixed 👌 Was actually caused by a previous PR but I've addressed it here since it was easy enough.

Copy link
Contributor

@deanhannigan deanhannigan left a comment

Choose a reason for hiding this comment

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

LGTM! The updates to the grid are solid! After a run through, I can see the context menu is back in place and no other issues remain 👍

@aptkingston aptkingston merged commit 39f59c0 into master May 20, 2024
10 checks passed
@aptkingston aptkingston deleted the table-width-setting branch May 20, 2024 10:05
@github-actions github-actions bot locked and limited conversation to collaborators May 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants