Updated Lazy document to handle viewing PDF's without downloading. #253
Conversation
| @@ -0,0 +1,329 @@ | |||
| /* Copyright 2014 Mozilla Foundation | |||
There was a problem hiding this comment.
I was able to find an example of another component that imports CSS from node_modules. Would this type of setup work (plus the customization in Webpack config?):
There was a problem hiding this comment.
I will give it a try. I will need to publish a new version to test
There was a problem hiding this comment.
That did work. Updating.
| @@ -25,7 +29,7 @@ const FacetSlider = forwardRef(({ useRangeSlider, ...props }: Props, ref: HTMLEl | |||
| refine, | |||
| } = useRangeSlider(props); | |||
|
|
|||
| const [valueView, setValueView] = useState<Array<number>>([range.min, range.max]); | |||
| const [valueView, setValueView] = useState < Array < number >> ([range.min, range.max]); | |||
There was a problem hiding this comment.
Can we preserve the current spacing rules? I'm not sure if this is a Prettier/ESLint issue, or something else.
| @@ -72,12 +76,12 @@ const FacetSlider = forwardRef(({ useRangeSlider, ...props }: Props, ref: HTMLEl | |||
| columns={2} | |||
| > | |||
| <Grid.Column> | |||
| { valueView[0] } | |||
| {valueView[0]} | |||
There was a problem hiding this comment.
Do we have a style guide I can use in VSCode? This I believe is the default ruleset of the VSCode formatter it is using as I do not have prettier activated right now...Though I think we should use it ;-)
| @@ -26,6 +31,8 @@ type Props = { | |||
| image?: any, | |||
| pdf?: boolean, | |||
| preview?: ?string, | |||
| view?: Boolean, | |||
| view_url?: String, | |||
There was a problem hiding this comment.
Let's use camel-case prop names for consistency with other components.
| * | ||
| * @type {string} | ||
| */ | ||
| const className = useMemo(() => { |
There was a problem hiding this comment.
Most of these props can be passed to the Button component as props, instead of concatenting them as a class name. I'm wondering if this needs to be it's own component or can be done inline in the LazyDocument:
<Button
basic
content={...}
primary
icon='file pdf'
onClick={...}
/>
There was a problem hiding this comment.
This is a copy/paste of the DownloadButton component as a starting point. I switched from using an anchor to see if I could resolve an earlier issue. I use this button outside of LazyDocument in the ArchNet client so I think it makes sense to keep it as a functional component, but I could definitely refactor the classname stuff. Do not know if it is a priority right now.
In this PR