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

Storybook home page is showing reference image at 2x size #943

Closed
nate-ni opened this issue Jan 5, 2023 · 1 comment · Fixed by #1000
Closed

Storybook home page is showing reference image at 2x size #943

nate-ni opened this issue Jan 5, 2023 · 1 comment · Fixed by #1000
Labels
bug Something isn't working

Comments

@nate-ni
Copy link
Contributor

nate-ni commented Jan 5, 2023

🐛 Bug Report

The image on the home page is too large.

💻 Repro or Code Sample

image

🤔 Expected Behavior

The image should display at a normal size

😯 Current Behavior

It is larger (2x, I think)

💁 Possible Solution

🔦 Context

🌍 Your Environment

  • OS & Device: MacOs
  • Browser Chrome
@nate-ni nate-ni added bug Something isn't working triage New issue that needs to be reviewed labels Jan 5, 2023
@jattasNI
Copy link
Contributor

jattasNI commented Jan 5, 2023

@nate-ni I think this is the same issue as #825

@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Jan 10, 2023
rajsite added a commit that referenced this issue Jan 30, 2023
# Pull Request

## 🤨 Rationale

Updates our dependencies and devDependencies for all packages. The
intention is to not introduce any breaking changes and relying on tests
+ storybook for that.

Also adds a size to the storybook landing page image to fix #943 (we are
on the latest MDX2 now which is what changed for StoryBook 7 so I don't
think it's gonna change anything else related to MDX significantly).

Updates to the latest Angular 14 (but not up to 15).
Does not update TypeScript past 4.6:
- upgrading to 4.7 isn't required by anything and all our existing apps
are on 4.6, so didn't seem worth including in this PR. Maybe a follow-up
if it works without errors. Typescript 4.7 is needed for package
[import/export
support](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-7.html#packagejson-exports-imports-and-self-referencing).
- upgrading to 4.8 is desired as it is the minimum required by Angular
15 but is not supported by fast:
microsoft/fast#6365
- upgrading to 4.9 is not supported by Angular 14, Angular 15.1 is the
[first version to support
4.9](angular/angular@dd42974)

## 👩‍💻 Implementation

Result of `npm outdated` after changes:
```
Package                                  Current         Wanted         Latest  Location                                        Depended by
@angular-devkit/build-angular            14.2.10        14.2.10         15.1.3  node_modules/@angular-devkit/build-angular      angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/animations                      14.2.12        14.2.12         15.1.2  node_modules/@angular/animations                angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/cli                             14.2.10        14.2.10         15.1.3  node_modules/@angular/cli                       angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/common                          14.2.12        14.2.12         15.1.2  node_modules/@angular/common                    angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/common                          14.2.12        14.2.12         15.1.2  node_modules/@angular/common                    nimble-angular@npm:@ni/nimble-angular@16.0.3
@angular/compiler                        14.2.12        14.2.12         15.1.2  node_modules/@angular/compiler                  angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/compiler-cli                    14.2.12        14.2.12         15.1.2  node_modules/@angular/compiler-cli              angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/core                            14.2.12        14.2.12         15.1.2  node_modules/@angular/core                      angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/core                            14.2.12        14.2.12         15.1.2  node_modules/@angular/core                      nimble-angular@npm:@ni/nimble-angular@16.0.3
@angular/forms                           14.2.12        14.2.12         15.1.2  node_modules/@angular/forms                     angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/forms                           14.2.12        14.2.12         15.1.2  node_modules/@angular/forms                     nimble-angular@npm:@ni/nimble-angular@16.0.3
@angular/platform-browser                14.2.12        14.2.12         15.1.2  node_modules/@angular/platform-browser          angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/platform-browser-dynamic        14.2.12        14.2.12         15.1.2  node_modules/@angular/platform-browser-dynamic  angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/router                          14.2.12        14.2.12         15.1.2  node_modules/@angular/router                    angular-workspace@npm:@ni-private/angular-workspace@1.0.0
@angular/router                          14.2.12        14.2.12         15.1.2  node_modules/@angular/router                    nimble-angular@npm:@ni/nimble-angular@16.0.3
@tanstack/virtual-core             3.0.0-beta.41  3.0.0-beta.41  3.0.0-alpha.1  node_modules/@tanstack/virtual-core             nimble-components@npm:@ni/nimble-components@18.0.3
ng-packagr                                14.2.2         14.2.2         15.1.1  node_modules/ng-packagr                         angular-workspace@npm:@ni-private/angular-workspace@1.0.0
typescript                                 4.6.4          4.6.4          4.9.4  node_modules/typescript                         xliff-to-json-converter@npm:@ni/xliff-to-json-converter@1.1.0
typescript                                 4.6.4          4.6.4          4.9.4  node_modules/typescript                         nimble-tokens@npm:@ni/nimble-tokens@4.3.2
typescript                                 4.6.4          4.6.4          4.9.4  node_modules/typescript                         nimble-components@npm:@ni/nimble-components@18.0.3
typescript                                 4.6.4          4.6.4          4.9.4  node_modules/typescript                         angular-workspace@npm:@ni-private/angular-workspace@1.0.0
typescript                                 4.6.4          4.6.4          4.9.4  node_modules/typescript                         site@npm:@ni-private/site@1.0.0
zone.js                                   0.11.8         0.11.8         0.12.0  node_modules/zone.js                            angular-workspace@npm:@ni-private/angular-workspace@1.0.0
```


## 🧪 Testing

Rely on CI and chromatic

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
@jattasNI jattasNI mentioned this issue Mar 29, 2023
4 tasks
jattasNI added a commit that referenced this issue Mar 31, 2023
# Pull Request

## 🤨 Rationale

We want to be on the latest storybook version because it fixes issues
we've had with dependencies and it enables more powerful docs pages via
mdx and Component Story Format enhancements.

Notable changes:
- landing page uses correct fonts and errors in console are gone (fixing
#825 and #943 properly)
- [no more Docs
tab](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#70-docs-changes).
Each story group has a docs page under it instead.
- XD previews are gone (feature was deprecated)
- Actions not working yet and some code previews have stopped working;
see "Follow ups" below

## 👩‍💻 Implementation

Following the [upgrade
guide](https://storybook.js.org/docs/react/configure/upgrading#prereleases)
didn't work very well because it is buggy with a monorepo. Fred's
general strategy was to run the upgrade command from within
`nimble-components` then delete the `node_modules` and
`package-lock.json` that it creates there, then re-install everything
from the root. The migration tool is responsible for the changes to:
1. storybook configuration files
2. package.json
3. renaming types like `Story` -> `StoryFn` and `storyName` to `name`
4. deleting the `withXD` decorator which is no longer supported

Beyond that there were a few other manual changes:
1. establish static dependencies on components needed by each story.
Storybook seems to have gotten more aggressive about compiling away
unneeded components so if something isn't imported, it won't be found
and components won't render correctly. This meant:
1. replacing a few literals for element tag names in stories with the
`elementTag` export
2. updating the `nimble-menu` to explicitly depend on
`nimble-anchored-region` (see comment on `menu/index.ts`)
2. Enable 'autodocs' for component and token stories but disable it for
test stories
3. Add a patch file to work around
storybookjs/storybook#21820
4. simplified return types of story generation functions in
`storybook.ts` in response to storybook type changes
5. regenerated landing page screenshot and annotation now that docs tab
is gone

## 🔜 Follow ups in future PRs

Fixing these issues is tracked by #1146 . I tried upgrading to ts 4.7
but it didn't fix any of them.

- [ ] storybookjs/storybook#21820
- [ ] Figure out why actions aren't firing when you click a button.
Likely [need this
decorator](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#removed-auto-injection-of-storybookaddon-actions-decorator)
but it's not present in current rc:
storybookjs/storybook#21308
- [ ] Some code previews (table, tooltip) are not available in the Docs
page

## 🧪 Testing

Chromatic caught several diffs which resulted in the manual changes
described above 👍. ~~It's still showing new stories for the wafer map
and button. (The wafer map one is caused by Chromatic getting confused
when I tried renaming that story, ran a Chromatic build, and then
reverted that change. I don't know the cause of the button change.)~~ <-
this went away on a subsequent build

Also manually spot checked many of the stories across browsers,
especially ones that have workarounds in them like table and theme-aware
tokens.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Fred Visser <1458528+fredvisser@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

3 participants