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

Feature [RM67] Detail View Short Description #69

Merged
merged 3 commits into from
Aug 9, 2021

Conversation

swg99
Copy link
Contributor

@swg99 swg99 commented Aug 6, 2021

https://github.com/orgs/novoda/projects/1#card-66365504
The short and full descriptions are shown in the detail view when available.

Short descriptions are shown in cells instead of full descriptions if they are available.
Short description is shown with the full description when available.
@swg99 swg99 linked an issue Aug 6, 2021 that may be closed by this pull request
@@ -30,7 +34,12 @@ struct CharacterDetailView: View {

struct CharacterDetailView_Previews: PreviewProvider {
static var previews: some View {
CharacterDetailView(character: ricks[0])
CharacterDetailView(character: morties[0])
NavigationView {

Choose a reason for hiding this comment

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

I like the usage of previews! :)

Comment on lines +21 to +24
VStack(alignment: .leading, spacing: 12) {
if let c = character as? ShortCharacterDescription {
Text(c.shortDescription)
}

Choose a reason for hiding this comment

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

This if this is a duplicated logic, IF there would be 3rd way of describing character, like fullDescription You would have fix this in 2 places in code. If the application would have more then those two, would you put that logic everywhere?

@swg99 swg99 merged commit 400b248 into scottie-main Aug 9, 2021
@swg99 swg99 deleted the feature/rm-67-detail-view-short-description branch August 9, 2021 13:16
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.

Add short description to detail view
2 participants