-
Notifications
You must be signed in to change notification settings - Fork 9
feat(Gallery): add a hook for opening the gallery from the custom content #275
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
base: main
Are you sure you want to change the base?
Conversation
Preview is ready. |
ac0cb65
to
156b01e
Compare
06cd44a
to
1e4d437
Compare
src/hooks/useGallery/GalleryWithTheme/GalleryWithTheme.lazy.tsx
Outdated
Show resolved
Hide resolved
1e4d437
to
a29f63a
Compare
a29f63a
to
8ff4fe5
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.
Let's name this file AsyncGallery.tsx
to distinguish from normal Gallery component on search
import * as React from 'react'; | ||
|
||
export const Gallery = React.lazy(() => | ||
import('../../components/Gallery/Gallery').then((module) => ({default: module.Gallery})), |
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.
Is default export necessary? We only use named exports through the codebase.
|
||
The hook for opening the gallery | ||
|
||
### PropTypes |
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.
Let's keep this section at the end of readme, and name it Properties
|
||
| Property | Type | Required | Values | Default | Description | | ||
| :----------- | :---------------------------- | :------- | :----- | :------ | :------------------ | | ||
| theme | `ThemeProviderProps['theme']` | | | `dark` | The gallery theme | |
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.
dark
is no longer default, let's keep it blank, since it's inherited from the root ThemeProvider
| theme | `ThemeProviderProps['theme']` | | | `dark` | The gallery theme | | ||
| className | `String` | | | | The modal class | | ||
| container | `HTMLElement` | | | | The modal container | | ||
| emptyMessage | `String` | | | No data | No data message | |
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.
| emptyMessage | `String` | | | No data | No data message | | |
| emptyMessage | `String` | | | "No data" | No data message | |
|
||
_GalleryContextProvider_: | ||
|
||
| Property | Type | Required | Values | Default | Description | |
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.
Values column seems to be unnecessary
> & | ||
Pick<ThemeProviderProps, 'theme'>; | ||
|
||
export const GalleryContextProvider = ({ |
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.
Let's omit Context
from the name, just GalleryProvider
import {GalleryContext} from './GalleryContext'; | ||
|
||
export function useGallery() { | ||
return React.useContext(GalleryContext).openGallery; |
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.
Maybe return whole context value here in case of extending API in the future?
A hook for opening the gallery from the custom content