-
Notifications
You must be signed in to change notification settings - Fork 21
feat(elements): add file picker #3404
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
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit c130092. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution
✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
9d75f77 to
ad661bd
Compare
|
It seems like we cannot update the screenshots anymore due to Nx Cloud:
|
You still can. Nx cloud is IMO not very friendly for open-source projects. Everything still works the same. You can update screenshots by running the workflow action on your branch. You might have to bring your branch up to date with master first |
de0f8ae to
8bdd687
Compare
BundleMon (elements)Files updated (3)
Total files change +29.75KB +4.73% Final result: ✅ View report in BundleMon website ➡️ |
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/test/unit/components/file-picker.component.spec.ts
Outdated
Show resolved
Hide resolved
5724665 to
18937df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played around with it a little bit, but I think it doesn't fully work as it should yet.
I'm also wondering if you could get nice behaviour by default by using the html dialog element
1: Opening the search icon and pressing Enter right away navigates to the first file, but doesn't dismiss the file picker. Also, I think opening the file-picker should focus the search-box. You can use the property autofocus on the input element for that.
Screen.Recording.2024-11-12.at.12.50.08.mov
2: Navigation doesn't always work with relative url's. Not sure why this works in some reports, but not in others.
Screen.Recording.2024-11-12.at.12.51.58.mov
3: Partial search does not work as expected. Ideally the search would be case-insensitive, too
Screen.Recording.2024-11-12.at.12.54.24.mov
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
...hots/Drawer-mutant-view-when-a-mutant-is-opened-should-look-as-expected-1-chromium-win32.png
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
551d508 to
5afe75d
Compare
|
Should be fixed now, I tried using the |
497adbe to
324d64c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the back-and-forth with review comments. I took a bit of a closer look this time. If everything is resolved I think you can merge this. It's an awesome feature!
I think the dialog should be a separate experiment PR
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/test/unit/components/breadcrumb.component.spec.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
test(screenshots): update screenshots for Linux test(screenshots): update screenshots for Windows
8c5febd to
fe9c225
Compare
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/src/components/file-picker/file-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/elements/test/unit/components/breadcrumb.component.spec.ts
Outdated
Show resolved
Hide resolved
hugo-vrijswijk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Great work!
Adds a file picker to the report, which allows for easier keyboard traversing over the report.
related #3370
ToDo: