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

Add default failbacks #37

Merged
merged 6 commits into from
Jun 26, 2020
Merged

Add default failbacks #37

merged 6 commits into from
Jun 26, 2020

Conversation

paltman
Copy link
Contributor

@paltman paltman commented Jun 24, 2020

No description provided.

@paltman
Copy link
Contributor Author

paltman commented Jun 24, 2020

Copy link
Contributor

@jacobwegner jacobwegner left a comment

Choose a reason for hiding this comment

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

@paltman I think you included portions of #30 in this PR (likely to pick up adding the fallback to the widget). I'm about to approve #30 so no harm no foul.

src/widgets/AudioWidget.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@jacobwegner jacobwegner left a comment

Choose a reason for hiding this comment

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

@paltman This is another thing where I feel like our acceptance criteria in Trello is not thorough enough.

Looking at
https://deploy-preview-37--explorehomer-dev.netlify.app/reader?urn=urn%3Acts%3AgreekLit%3Atlg0012.tlg001.msA-folios%3A12r.1.1-12r.1.25, we have better fallbacks on the widgets:

image

But not on the display mode (e.g. sentence alignments here):

image

or the metrical annotations:

image

(We do have a good fall back on the folio images though already)

@paltman paltman requested a review from jacobwegner June 25, 2020 20:53
@jtauber
Copy link
Member

jtauber commented Jun 25, 2020

The first thing that's clear is that we're completely inconsistent in the styling of the widget fallback.

  • different font size
  • different indentation
  • "No X available" vs "No Y" vs "No Z found"
  • period vs no period

My slight preference is the Named Entities variant. I certainly prefer the smaller font size, less indentation, and no period.

Not sure what which I prefer between "available" or "found" or neither.

@paltman
Copy link
Contributor Author

paltman commented Jun 25, 2020

I'll make a component called <EmptyMessage> that can have it's own single style.

@paltman
Copy link
Contributor Author

paltman commented Jun 25, 2020

@jtauber everything is now the same font/size/padding/message. It's a default message in a slot so we can override with specific language. Just let me know what language you want in what widget/place.

@jtauber
Copy link
Member

jtauber commented Jun 26, 2020

For the side widgets, I'd drop the text-align: center; and change the padding to padding: 0 0 0.4rem;

I understand the centering and margins are needed when in the reader widget (metrical annotation and folios) but in side widgets they take up a bit too much space and draw too much attention.

@paltman
Copy link
Contributor Author

paltman commented Jun 26, 2020

@jtauber fixed

# Conflicts:
#	src/reader/components/ReaderView.vue
#	src/widgets/WordListWidget.vue
@jacobwegner jacobwegner merged commit e425e29 into develop Jun 26, 2020
@jacobwegner jacobwegner deleted the 170-default branch June 26, 2020 19:53
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.

3 participants