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

Add a 'Sellable' list #16

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add a 'Sellable' list #16

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f1e344
ItemList refactored and sellable list added
trackness Jul 6, 2022
d8b0127
WebStorm told me to fix these sorry
trackness Jul 6, 2022
9852c2e
icon consistency
trackness Jul 6, 2022
ceed381
pr change requests
trackness Jul 6, 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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Consider supporting me via [Patreon](https://www.patreon.com/Vedgy)

## Pull Requests and issues

Feel free to submit pull requests if you know your way around JSON/React and Github issues if you don't.
Feel free to submit pull requests if you know your way around JSON/React and GitHub issues if you don't.
MatthewSbar marked this conversation as resolved.
Show resolved Hide resolved

## Legal Stuff

Expand Down
3 changes: 1 addition & 2 deletions src/App.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,9 @@ footer {
.patreon-link {
border-radius: 8px;
background-color: #525276;
margin: 0 16px 0 0;
padding: 8px 16px;
font-size: 16px;
margin-bottom: 16px;
margin: 0 16px 16px 0;
}

.patreon-link {
Expand Down
136 changes: 79 additions & 57 deletions src/components/item-list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { quests } from "../data";
import { ItemName, ItemSource, QuestProgress } from "../types";

const deadDrop = "💀🗑️";
trackness marked this conversation as resolved.
Show resolved Hide resolved

type Props = {
itemsNeeded: Record<ItemName, number>;
search: string;
Expand All @@ -18,68 +20,88 @@ export const ItemList = ({
focusQuests,
omittedItems,
}: Props) => {
return (
<div>
<div className="search-wrapper">
<input
type="text"
onChange={(e) => setSearch(e.target.value)}
placeholder="Search for items"
/>
</div>
<div>💀🗑️ = Item must be dead dropped</div>
<hr />
const searchCount = search.length;
const itemsNeededCount = Object.keys(itemsNeeded).length;
const itemsCompletedCount = Object.keys(itemsNeeded).filter((key) => itemsNeeded[key as ItemName] === 0).length;
const focusQuestsCount = focusQuests.length;

{omittedItems === "quest" ? (
<div>Items needed for quests are currently hidden.</div>
) : focusQuests.length > 0 ? (
<div>
Only showing items from:
<ul className="quest-list">
{focusQuests.map((questName) => (
<li key={questName}>
{questName} - part {questProgress[questName] + 1}
</li>
))}
</ul>
</div>
) : null}
{omittedItems === "upgrade" ? (
<div>Items needed for upgrades are currently hidden.</div>
) : null}
{omittedItems ||
(omittedItems === "quest" && focusQuests.length > 0 && <hr />)}
const searchBox = (<div className="search-wrapper">
<input
type="text"
onChange={(e) => setSearch(e.target.value)}
placeholder="Search for items"
/>
</div>);

const sellableList = (<div>
<hr />
<div>💰 Sellable Items</div>
trackness marked this conversation as resolved.
Show resolved Hide resolved
<hr />
{Object.keys(itemsNeeded)
.filter((key) => itemsNeeded[key as ItemName] > 0)
.filter((key) => itemsNeeded[key as ItemName] === 0)
.sort()
.map((key) => {
if (
search.length === 0 ||
key.toLocaleLowerCase().includes(search.toLocaleLowerCase())
) {
return (
<div key={key}>
{key}: {itemsNeeded[key as ItemName]}{" "}
{quests
.filter(
(quest) => quest.parts.length > questProgress[quest.name]
)
.some((quest) =>
quest.parts.some((part) =>
part.dropItems?.some((item) => item.item === key)
)
)
? "💀🗑️"
: null}
</div>
);
if (searchCount === 0 || key.toLocaleLowerCase().includes(search.toLocaleLowerCase())) {
trackness marked this conversation as resolved.
Show resolved Hide resolved
return (<div key={key}>{key}</div>);
}
})}
{Object.keys(itemsNeeded).filter(
(key) => itemsNeeded[key as ItemName] > 0
).length === 0
? "No items needed based on current filter criteria."
: null}
})
}
</div>);

const header = (<div>
<hr />
<div>{deadDrop}️ = Item must be dead dropped</div>
{omittedItems !== null ? (<div>
<div>{omittedItems.charAt(0).toUpperCase() + omittedItems.slice(1)} items are currently hidden.</div>
</div>) : null}
<hr />
</div>);

const focusedQuests = (<div>
<hr />
Only showing items from:
<ul className="quest-list">
{focusQuests.map((questName) => (
<li key={questName}>
{questName} - part {questProgress[questName] + 1}
</li>
))}
</ul>
</div>);

const itemList = (<div>
{Object.keys(itemsNeeded)
.filter((key) => itemsNeeded[key as ItemName] > 0)
.sort()
.map((key) => {
if (searchCount === 0 || key.toLocaleLowerCase().includes(search.toLocaleLowerCase())) {
return (
<div key={key}>
{key}: {itemsNeeded[key as ItemName]}{" "}
{quests
.filter((quest) => quest.parts.length > questProgress[quest.name])
.some((quest) =>
quest.parts.some((part) =>
part.dropItems?.some((item) => item.item === key)
)
)
? deadDrop
: null}
</div>
);
}
})}
</div>);

return (
<div>
{searchBox}
{itemsCompletedCount === itemsNeededCount ? sellableList : (<div>
{header}
{focusQuestsCount > 0 ? focusedQuests : null}
{itemList}
{focusQuestsCount === 0 && omittedItems === null ? sellableList : null}
</div>)}
Copy link
Owner

Choose a reason for hiding this comment

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

I can't decide if I like this way of organizing elements. Part of me feels like if you're going to separate all of them, you may as well make them components. Mind convincing me this is the way to go? I agree it's generally more readable than just having all the markup loose, but, what are your thoughts on taking the elements here and making them components versus doing it this way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No argument whatsoever, really. This is my first foray into React; I mostly do backend Go/Python so if anything it was to help myself understand the current structure and learn how to refactor etc. In that spirit, I'm probably in favour of whatever allows maintainability and extensibility. I can see why that might be more granular components, if appropriate.

Copy link
Owner

Choose a reason for hiding this comment

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

Gotcha. Well I'm impressed, and I was actually going to reach out and ask if you were looking for a job at all, I need to hire a senior FE dev, I love your attention to detail already.

About the components, I know some people will do this kind of thing, I'm not used to it, but that doesn't mean it's wrong. The beauty/ugly of react is you can kinda do whatever and none of it is "wrong". I'm okay with leaving it as-is but if these elements were reused later I think I'd rather see them as components. I might prefer to see them as components anyways just so this file doesn't get too big, as it kind of is, but, having a ton of files with a few lines isn't always helpful either. We can just run with it and switch it up later if it's a problem. I think this is actually a pretty sensible way to organize these elements within a component and I'm just experiencing an existential struggle as I challenge my worldview :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's always nice to know I'm not the only one learning something through a PR! The apparent structural flexibility of React (JS/TS, really) is quite a shift from what I'm used to, so knowing the pros and cons of a situation is super helpful, thank you.

If the current state is or gets too unwieldy, refactoring into components sounds like another learning opportunity, so raise an issue if so and I'll give it a spin.

As for the offer, I'm very flattered but in a good spot at the moment I think. You never know what's around the corner though 🤷

</div>
);
};
8 changes: 4 additions & 4 deletions src/types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ export type ViewMode = "quest" | "upgrade" | "items";
export type ItemSource = "quest" | "upgrade" | null;

type Part = {
dropItems?: ItemHandin[];
deliverItems?: ItemHandin[];
dropItems?: ItemHandIn[];
deliverItems?: ItemHandIn[];
description?: string;
};

export type ItemHandin = {
export type ItemHandIn = {
item: ItemName;
quantity: number;
dropLocation?: DropLocation;
Expand Down Expand Up @@ -137,7 +137,7 @@ type UpgradeTree =
| "Quarters";

type UpgradeLevel = {
items: ItemHandin[];
items: ItemHandIn[];
kMarks: number;
};

Expand Down