[PE-389-1][PE-1517] chore: remove the domain prop #51
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CI:
Adding the following ENV variable fixes this issue. NODE_OPTIONS=--openssl-legacy-provider |
luqven
changed the title
luqven
changed the title
luqven
changed the title
This was referenced Oct 28, 2021
This commit removes the `domain` prop from the `Imgix` component to fix an issue with the types. The `@types/react-imgix` repo doesn't yet have the new `domain` prop defined.
luqven
force-pushed
the
luis/apiKeyErrors
branch
from
b096d80
to
ddcccda
Compare
luqven
force-pushed
the
luis/removeDomainProp
branch
from
4b5d6e7
to
b3049cd
Compare
frederickfogerty
approved these changes
Nov 1, 2021
| @@ -14,8 +14,7 @@ export function AssetGrid({ assets, domain }: Props): ReactElement { | |||
| <div className="ix-grid-item" key={`${asset.id}-${idx}`}> | |||
| <div className="ix-grid-item-image"> | |||
| <Imgix | |||
| domain={domain} | |||
| src={asset.attributes.origin_path} | |||
| src={"https://" + domain + asset.attributes.origin_path} | |||
There was a problem hiding this comment.
I assume so, but just checking: does asset.attributes.origin_path always start with a /?
There was a problem hiding this comment.
AFAIK yes. I've yet to run into an instance where that's not the case. We might want to "normalize" these paths in future to play it safe.
|
Just FYI, this should be working now DefinitelyTyped/DefinitelyTyped@59ee82d |
This commit creates the `imgixApi.search` method that allows the searchbar component to search for assets. This is a placeholder endpoint, as in a future commit this should get refactored to be cleaner and less error prone. Ideally, we'd actually hit a `/search` endpoint rather than use the same valye for tags, filename, etc to filter.
This commit adds `onSubmit` and `onChange` handlers to the `SearchBar` component. This is necessary in order to change this form into a controlled form. Moreover, it makes integration of this component with custom handlers easier.
This commit adds search capabilities to the asset browser. The commit incldues an `onSearchSubmit` handler that should probably be refactored out of the component at a later date.
This commit adds a `placeholder` prop to the `AssetGrid` component. It also adds an `errors` state to the `AssetBrowser` component. The `imgixAPI` error messages are passed to the `placeholder` prop when they occur to more easily inform the user of any API errors. The commmit also moves the loading spinner inside the grid. This makes it easier to pass the spinner as the `placeholder` prop whenever the `loading` state is `true`.
This commit adds the `placeholder` prop the story and fixes the `origin_path` value, since we can always assume it starts with a `/`.
This commit creates the `CursorT` type. The `cursor` is part of the `meta` value of the response object to an imgix API requet. The `cursor` holds pagination information about the assets requested. Whenever the page or cursor parameters are used to filter the assets, the cursor object updates to reflect the state of the asset pagination. For example, it holds the `cursor.next` index value, which typically the value of `cursor.current` plus the `limit` value passed to the pagination parameters.
This commit creates the `Pagination.tsx` component. The component renders two buttons that invoke the `handlePageChange` function prop when clicked. One button passes `-1` as an arugment and the other `1`, IE one decrements and the other incremenets the "page index". The buttons only render if the `cursor` prop has `totalRecords` that evaluate to `true`.
This commit stores the `response.meta.cursor` object as part of the returned object, `response.cursor`. This object stores asset pagination information. This commit also adds `index` and `size` as parameters with default values for the service modules `souces.assets` and `search` endpoints. These parameters allow the requests to filter results by page index and range.
This commit removes the use of `React.useState` from the component and instead lifts the component state into a container component, `AssetBrowserContainer`. The state-setters are accepted as props to this component, IE `setQuery`. The commit also removes the need for rendering the `AssetGrid` placeholder, and instead passes on that responsability to the `AssetGrid`. Finally, the commit creates a `handleSearch` function that invokes the `setLoading` and `setQuery` props and, if a source is currently selected, runs `requestAssetsFromSource` prop to search for assets using the `query` as a filter.
This commit adds `errors` and `loading` as accepted props in order to handle the placeholder rendering logic inside the component. This will probably be re-worked in a future commit, as this should really be handled further up the component strucutre. But, for now, this way of handling showing the loading spinner, placehilder, and errors is better than what was being done at the `AssetBrowser.tsx` level.
This commit creates the `AssetBrowserContainer` component. This component houses all the lifted states from `AssetBrowser` as one Class component state. Each slice of state that is passed as a prop has its own setter function that is also passed along as a prop. This component also houses all the api-wrapping functions for fetching and searching for assets. Each of those functions is also passed along as a prop to the `AssetGrid` component. An effort was made to make this into a functional component. However, I quickly discovered that created more "commits" in the react-dev-tools profiler. I'm therefore choosing to leave this as is despite the code-smell of having such a large state in this component.
This commit imports and uses the `Pagination` component to enable asset browser pagination. The component uses the pre-defined `handlePageChange` function to updae the cursor.
Co-authored-by: Frederick Fogerty <frederick.fogerty@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit removes the
domainprop from theImgixcomponent to fix an issue with the types. The@types/react-imgixrepo doesn't yet have the newdomainprop defined.Stacked PR Chain: PE-389-1