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.

Sign up for GitHub

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

react-components: Stop generating a folder with a file inside and instead flatten to a single file for 'Migrating from @fluentui/react v8' storybook story #21158

Merged
merged 3 commits into from
Jan 4, 2022
Merged

react-components: Stop generating a folder with a file inside and instead flatten to a single file for 'Migrating from @fluentui/react v8' storybook story #21158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d1de984
react-components: Stop generating a folder with a file inside and ins…
khmakoto Jan 4, 2022
105003f
Change files
khmakoto Jan 4, 2022
b51aea9
Updating change file.
khmakoto Jan 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@fluentui-react-components-059500c6-1617-4bd9-9f42-ea4726c5ece3.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "react-components: Stop generating a folder with a file inside and instead flatten to a single file for 'Migrating from @fluentui/react v8' storybook story.",
"packageName": "@fluentui/react-components",
"email": "[email protected]",
"dependentChangeType": "none"
}
2 changes: 1 addition & 1 deletion packages/react-components/.storybook/preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const options = {
[
'Introduction',
'Developer',
['Quick Start', 'Migrating from @fluentui/react v8', 'Styling Components', 'Positioning Components'],
['Quick Start', 'Migrating from @fluentui᜵react v8', 'Styling Components', 'Positioning Components'],
Copy link
Member

Choose a reason for hiding this comment

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

This is an interesting idea but I do worry a little about reducing searchability (if someone tries to find @fluentui/ it won't come up with anything), and about someone not realizing why it's a weird slash and changing it back to a normal slash later. But based on the storybook docs it looks usage of a slash to trigger grouping is hardcoded on their side, so I'm not sure what else we could do unless they provide a hook to modify grouping behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I found the same documentation and also found this thread where they specify that they don't support escaping characters, so I don't know what else we could do.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can also add a comment indicating why it's a weird slash if that sounds helpful.

Copy link
Member

Choose a reason for hiding this comment

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

Huh okay, in that case let's go with the weird slash and add a comment

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you forgot to add the reasoning into the source as the PR was merged. can you add it please ?

Copy link
Contributor

@Hotell Hotell Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

in general I'm not a fan of this "hack".

  • should this document be under Migrations ?
    2022-01-05 at 13 32

  • cant we have the menu item named as Migrating from @fluentui react v8 until SB implements proper slash escaping ? From my perspective anyone that uses v8 (fluentui/react) knows what v8 means and thus missing exact package name in the menu navigation shouldn't be an issue. WDYT ?

Copy link
Member

Choose a reason for hiding this comment

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

Saying Migrating from @fluentui react v8 could be okay, since "v8" is used alongside the name of the library. But in general we need to be careful about using "v8" as a standalone shorthand in user-facing content since it's also the name of a javascript engine (despite the very different context, it's confused some people in the past).

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good, thx for additional context !

],
'Theme',
'Components',
Expand Down
2 changes: 1 addition & 1 deletion packages/react-components/src/Concepts/MigratingFromV8.stories.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Meta } from '@storybook/addon-docs';

<Meta title="Concepts/Developer/Migrating from @fluentui/react v8" />
<Meta title="Concepts/Developer/Migrating from @fluentui᜵react v8" />

## Moving to `@fluentui/react-components` version 9-BETA from `@fluentui/react` version 8

Expand Down