-
-
Notifications
You must be signed in to change notification settings - Fork 644
feat: selecting Compare from item popup in Compare panel will set new initialItemId #10861
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: master
Are you sure you want to change the base?
Conversation
3165d4c
to
6efa696
Compare
I can see that you no longer can add a matching item by just clicking on it - you have to click "Compare" or hit C. That's fine, though it may confuse folks used to the old workflow. Hitting "Compare" on a non-matching item gives the same old error about not being comparable - it doesn't start a new compare session, and the label doesn't change. Am I missing something? |
I don't think either of the two versions of this PR (first commit starting new session or second commit setting new initialItemId but preserving session) had this behavior, but I think it's worth adding as the behavior when selecting a non-matching item. I'm leaving this in draft until I can implement that. |
readonly initialItemId?: string; | ||
initialItemId?: string; |
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.
Anything in the redux state has to be fully readonly
|
||
const openCompare = () => { | ||
hideItemPopup(); | ||
if (fromCompare && session) { | ||
session.initialItemId = item.id; |
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.
Same as the other comment - you can't mutate the session like this, you have to dispatch an action that creates a new session. Probably a different version of addCompareItem
.
… new session Text on button changes to New Compare when this will occur Adding items to Compare session from main inventory now requires selecting Compare instead of being one click
Text on button changes to New Compare when this will occur
Adding items to Compare session from main inventory now requires selecting Compare on Item Popup instead of being one click
First commit in this PR has the behavior of fully removing the existing compare session and starting a new one with the selected item.