Implementation of #101 - Close comparison buttons #123
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
regarding the #101
regarding the #101 Co-authored-by: Julian Oelhaf <julian.oelhaf@posteo.de>
Expanded state of DetailsComponent to have three variables for each possible state Made close buttons switch stats Made DetailsComponent switch content depending on the state Regarding #101 Co-authored-by: Irem Toroslu <irem.toroslu@fau.de>
structured and resized the navbar and the navbar icons fontsizes and fontweight changed. regarding the #101 Co-authored-by: Julian Oelhaf <julian.oelhaf@posteo.de>
Commented the changes made in the previous commit Documented the new functionality Regarding #101 Co-authored-by: Irem Toroslu <irem.toroslu@fau.de>
Waldleufer
requested changes
Jun 18, 2021
| @@ -71,7 +96,8 @@ class DetailsComponent extends Component { | |||
|
|
|||
| // postCalculationRequest(selectedProduct.productID); | |||
|
|
|||
| if (!this.state.loadComparePage) { | |||
| if (this.state.baselineScenario) { | |||
| // if state equals baseline scenario only | |||
There was a problem hiding this comment.
I think if you really want an XOR you should maybe change this. Because
const baselineScenario = true;
const modifiedScenario = true;
const loadComparePage = true;Would also activate this case e.g.
There was a problem hiding this comment.
The way it is rn: baselineScenario has top priority, then modified then loadCompare page. Intended?
There was a problem hiding this comment.
I really like how it is done btw :D
There was a problem hiding this comment.
Yeah, we haven't given it too much thought what should be top priority, because in reality if all three are true, then we have a problem :)
Co-authored-by: Irem Toroslu <irem.toroslu@fau.de>
regarding the #101 button classnames redefined and hand mouse point used
class -> className
To avoid using the reserved name "key"
manifau
reviewed
Jun 20, 2021
Put <a> in <div> to align them dynamically.
Waldleufer
approved these changes
Jun 22, 2021
Merge Preparation
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.
No description provided.