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.

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

Feedback Show #139

Merged
merged 5 commits into from
Jul 4, 2023
Merged

Feedback Show #139

merged 5 commits into from
Jul 4, 2023

Conversation

JoshDevHub
Copy link
Collaborator

@JoshDevHub JoshDevHub commented Jul 3, 2023

Issue

Closes #126

Description

  • Extracts content on Pair Request show into reusable component
  • Uses new component for Pair Request show and Feedback show (if shown feedback is associated with a pair request)

Screenshots

This is the same content that exists for Pair Request show

Mobile

Selection_198

Desktop

Selection_197

Additional Comments

  • I think it's probably best to hold off on making this work with User feedback until user feedback is actually implemented as a feature. We could go ahead and do this if we decide that we want to.
  • Back linking is also a concern currently. If a user edits/updates feedback, they no longer have a clean way back to the index page from which they came from (browser back is insufficient). This can be solved, but I also think it's something to address with in the PR for feedback edit since the particulars may be dependent on how edit is implemented (ie. turbo stream vs. html response).

GALTdea and others added 5 commits June 28, 2023 12:19
component to avoid turbo behavior in the link.
Co-authored-by: Gustavo Valenzuela <gustavo.devvv@gmail.com>
Co-authored-by: Gustavo Valenzuela <gustavo.devvv@gmail.com>
Copy link
Collaborator

@toyhammered toyhammered left a comment

Choose a reason for hiding this comment

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

Love it! The one comment I do have is related to who the actual "owner" of this component is. Right now Pair Request owns this, but I am wondering if we want Feedback to be the actual owner? This is just an open question but I am totally good with this PR.

@JoshDevHub
Copy link
Collaborator Author

JoshDevHub commented Jul 4, 2023

Yeah that's an interesting question. I do think there are some component names that could be redone, but I might stand by this being under the PairRequest namespace.

This was originally designed for the pair request show page and shows all the information about the pair request itself + both feedback items associated with the pair request (if they exist). Those feedback sections are handled by the Feedback::ContainerComponent (this name should change I think), but I think this new component as a whole is more of a Pair Request thing than a Feedback thing. Willing to be swayed on that though.

@JoshDevHub JoshDevHub merged commit 678442e into main Jul 4, 2023
1 check passed
@JoshDevHub JoshDevHub deleted the feedback-show branch July 4, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feedback Epic] Feedback#show
3 participants