Skip to content

Add Family size table watching#236

Merged
Nice3point merged 10 commits intolookup-foundation:devfrom
SergeyNefyodov:dev-family-size-tables-support-add-table-view
May 14, 2024
Merged

Add Family size table watching#236
Nice3point merged 10 commits intolookup-foundation:devfrom
SergeyNefyodov:dev-family-size-tables-support-add-table-view

Conversation

@SergeyNefyodov
Copy link
Collaborator

@SergeyNefyodov SergeyNefyodov commented May 14, 2024

Summary of the Pull Request

What is this about:

I added a possibility to watch read-only FamilySizeTable by clicking context menu, using dialogs

Quality Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@SergeyNefyodov SergeyNefyodov changed the title Dev family size tables support add table view Add Family size table watching May 14, 2024
contextMenu.AddMenuItem()
.SetHeader("Show table")
.SetAvailability(table.IsValidObject)
.SetCommand(table, async _ =>
Copy link
Member

Choose a reason for hiding this comment

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

Pass sizeTable lambda params instead of discard param

.SetCommand(table, async **sizeTable** =>
{
...
   var dialog = new FamilySizeTableDialog(context.ServiceProvider, **sizeTable**);

Because you pass table as parameter to the command

Comment on lines 68 to 72
#if REVIT2022_OR_GREATER
unitsArray[i] = UnitUtils.IsUnit(typeId) ? typeId.ToUnitLabel() : typeId.ToGroupLabel();
#else
unitsArray[i] = UnitUtils.IsUnit(typeId) ? typeId.ToUnitLabel() : "Other";
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Instead of UnitUtils.IsUnit(typeId) you need to use typeId.Empty() check and remove REVIT2022_OR_GREATER directive

{
for (var i = 0; i < columnsCount; i++)
{
Columns.Add(new DataColumn(_table.GetColumnHeader(i).Name));
Copy link
Member

Choose a reason for hiding this comment

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

We can add unit in column header ? like .csv file?

    private void CreateColumns(FamilySizeTable table)
    {
        for (var i = 0; i < table.NumberOfColumns; i++)
        {
            var typeId = table.GetColumnHeader(i).GetUnitTypeId();
            var headerName = table.GetColumnHeader(i).Name;
            
            var columnName = typeId.Empty() ? headerName : $"{headerName}##{typeId.ToUnitLabel()}";
            Columns.Add(new DataColumn(columnName, typeof(string)));
        }
    }

<DataGrid
HorizontalScrollBarVisibility="Visible"
ItemsSource="{Binding}"
AutoGenerateColumns="True"/>
Copy link
Member

Choose a reason for hiding this comment

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

AutoGenerateColumns true by default, can remove it

CanContentScroll="True"
d:DataContext="{d:DesignInstance dialogs:FamilySizeTableDialogViewModel}">
<DataGrid
HorizontalScrollBarVisibility="Visible"
Copy link
Member

Choose a reason for hiding this comment

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

IsReadOnly="True"
HeadersVisibility="Column"
CanUserReorderColumns="False"

We can add those properties for better readability and remove scrollViewer because DataGrid has scroll by default in the template

Comment on lines 10 to 11
Height="Auto"
MinWidth="416"
Copy link
Member

Choose a reason for hiding this comment

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

we can remove it, the content takes up all available space

Title = "Family size table",
Content = this,
CloseButtonText = "Close",
DialogMaxWidth = 1500
Copy link
Member

Choose a reason for hiding this comment

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

I guess it's better to remove the max width if we want to see large horizontal tables in the full screen mode

@Nice3point
Copy link
Member

Nice3point commented May 14, 2024

@SergeyNefyodov I've cleaned the code a bit on the comments above, it looks like:
изображение
Compare with before:
изображение

Maybe the drawback is the Column1 name for the first column, but DataTable doesn't support empty name

@Nice3point Nice3point merged commit 08fa86b into lookup-foundation:dev May 14, 2024
@SergeyNefyodov SergeyNefyodov deleted the dev-family-size-tables-support-add-table-view branch May 18, 2024 07:55
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