Skip to content

Conversation

@jeffersonking
Copy link
Contributor

@jeffersonking jeffersonking commented Aug 17, 2018

Pull request checklist

  • Addresses an existing issue: Fixes DetailsList header and rows exceeds DetailsList #5857
  • Include a change request file using $ npm run change
    ERROR: Command failed: git diff upstream/master... --dirstat=files,0
    fatal: ambiguous argument 'upstream/master...': unknown revision or path not in the working tree.

Description of changes

DetailsList width calculation is not currently taking into account grouping depth >1. This fixes that.

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@JasonGore
Copy link
Member

Needs a change file.

@jeffersonking
Copy link
Contributor Author

Anyone have a tip on how to get npm run change working? Is there a way to point it to a commit? Thanks!

@JasonGore
Copy link
Member

If you run it from root directory on this PR's branch, it should automatically detect the changes for you... if it doesn't run correctly, you may have to run npm install to get the proper dependencies. If you still have trouble, I can generate it for you as well.

@jeffersonking
Copy link
Contributor Author

I got npm run change to work and attached is the output JSON. It doesn't seem to contain information that isn't available already. Is this correct? Thanks.

issue5857_2018-08-17-18-09.json.txt

@JasonGore
Copy link
Member

JasonGore commented Aug 17, 2018

Yep! It's just a delta regarding your change that's used for publishing. Just add and commit with name and location exactly as it was created (should be in a subdirectory under common.)

@jeffersonking
Copy link
Contributor Author

Thanks @JasonGore, it's been added.

@jeffersonking
Copy link
Contributor Author

@JasonGore or @KevinTCoughlin. Looks like this just needs a reviewer stamp and the "default" check to get approved? Any chance you guys could help? Thanks!

@KevinTCoughlin
Copy link
Member

@jeffersonking what's the best way to see a GroupedList with the width calculation issue? The change makes sense, but I'm trying to better understand why the nested example we have in the docs isn't affected by it since it is nested three levels deep. This Codepen seems the same between master and this branch -- https://codepen.io/kevintcoughlin/pen/pOoLJK?editors=1111

@jeffersonking
Copy link
Contributor Author

@KevinTCoughlin You need two things simultaneously: 1) 2+ levels of grouping depth, 2) header columns. The DetailsList docs don't have #1. The GroupedList docs don't have #2.

I can't speak for the Codepen but here is my repro:

import * as React from 'react'
import {DetailsList, DetailsListBase, ConstrainMode} from 'office-ui-fabric-react/lib/DetailsList'
import {SelectionMode} from 'office-ui-fabric-react/lib/utilities/selection/index'

export class Repro extends React.Component<any> {
	render() {
		const items = [{ },{ },]
		const groups = [
			{ key: 'a', name: 'Aaa', level: 0, startIndex: 0, count: 2, children: [
				{ key: 'b', name: 'Baa', level: 1, startIndex: 0, count: 2 }
			] }
		]
		const columns = [
			{ minWidth: 300, key: 'snippet',  name: 'Details' },
			{ minWidth: 120, key: 'build',    name: 'First discovered'},
			{ minWidth: 80,  key: 'bug',      name: 'Bug'},
			{ minWidth: 60,  key: 'hitcount', name: 'Hit Count'},
		].map((col: any) => ({
			...col,
			onRender: (item => 'Lorem ipsum dolor sit amet'),
		}))

		return <DetailsList
			items={items}
			groups={groups}
			groupProps={{ showEmptyGroups: true, }}
			constrainMode={ConstrainMode.unconstrained}
			compact={true}
			columns={columns}
			selectionMode={SelectionMode.multiple}
			/>
	}
}

Copy link
Member

@KevinTCoughlin KevinTCoughlin left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and the repro snippet @jeffersonking.

Tested the following in Chrome, FF, and Edge:

  • Repro case w/ & w/o column resizing enabled.
  • Existing DetailsList and GroupedList examples.

:shipit:

@kkjeer kkjeer merged commit 5de5881 into microsoft:master Aug 22, 2018
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
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.

DetailsList header and rows exceeds DetailsList

4 participants