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

Design/mobile fixes #509

Merged
merged 8 commits into from Jan 18, 2018
Merged

Design/mobile fixes #509

merged 8 commits into from Jan 18, 2018

Conversation

kristofferjs
Copy link
Contributor

No description provided.

@@ -203,6 +203,16 @@ export default class ReferenceInput extends React.Component<Props, State> {
const hasRef = value && value._ref
const hasWeakMismatch = hasRef && !isMissing && weakIs !== weakShouldBe

let inputValue
Copy link
Member

Choose a reason for hiding this comment

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

For readability, maybe you could create a function that takes the preview snapshot and returns the input value?

function resolveInputValue(snapshot) {
  if (snapshot === MISSING_SNAPSHOT) {
    return '<Unpublished or missing document>'
  }
  return snapshot.title || 'Untitled document'
}

// and then when passing the prop:
inputValue={resolveInputValue(previewSnapshot)}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried it, but it always returns 'untitled document' when the component should show the placeholder.

Copy link
Member

Choose a reason for hiding this comment

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

Weird. It should be equivalent, except that it should probably be a guard against snapshot being null. Let me fix it.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, the else-if block here will never evaluate as the first if block evaluates to true if previewSnapshot is MISSING_SNAPSHOT. Something was wonky with the indentation here too. Will fix in next

Copy link
Member

Choose a reason for hiding this comment

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

You were right, ofc, cause my suggestion did not consider if the input had a value.

Not entirely certain it became more readable, but at least it's fixed in next (5f2a79b) (squashed my patch into yours in a rebase, together with a fixup of the commit message)

Copy link
Member

@bjoerge bjoerge left a comment

Choose a reason for hiding this comment

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

Got one comment, otherwise looks good. Nice work!

@kristofferjs kristofferjs merged commit 1990b92 into next Jan 18, 2018
@kristofferjs kristofferjs deleted the design/mobile-fixes branch January 18, 2018 14:35
bjoerge pushed a commit that referenced this pull request Jan 18, 2018
* [components] Better dialog for mobile

* [components] Removing border on default pane

* [design] Styling button collections

* [design] Styling datepicker

* [design] General mobile fixes

* [components] Searchable select. Arrow down and handle value with no title

* [design] Mobile fixes

* [components] General padding of functions
bjoerge pushed a commit that referenced this pull request Jan 19, 2018
* [components] Better dialog for mobile

* [components] Removing border on default pane

* [design] Styling button collections

* [design] Styling datepicker

* [design] General mobile fixes

* [components] Searchable select. Arrow down and handle value with no title

* [design] Mobile fixes

* [components] General padding of functions
bjoerge pushed a commit that referenced this pull request Jan 24, 2018
* [components] Better dialog for mobile

* [components] Removing border on default pane

* [design] Styling button collections

* [design] Styling datepicker

* [design] General mobile fixes

* [components] Searchable select. Arrow down and handle value with no title

* [design] Mobile fixes

* [components] General padding of functions
bjoerge pushed a commit that referenced this pull request Jan 25, 2018
* [components] Better dialog for mobile

* [components] Removing border on default pane

* [design] Styling button collections

* [design] Styling datepicker

* [design] General mobile fixes

* [components] Searchable select. Arrow down and handle value with no title

* [design] Mobile fixes

* [components] General padding of functions
bjoerge pushed a commit that referenced this pull request Jan 28, 2018
* [components] Better dialog for mobile

* [components] Removing border on default pane

* [design] Styling button collections

* [design] Styling datepicker

* [design] General mobile fixes

* [components] Searchable select. Arrow down and handle value with no title

* [design] Mobile fixes

* [components] General padding of functions
bjoerge pushed a commit that referenced this pull request Jan 28, 2018
* [components] Better dialog for mobile

* [components] Removing border on default pane

* [design] Styling button collections

* [design] Styling datepicker

* [design] General mobile fixes

* [components] Searchable select. Arrow down and handle value with no title

* [design] Mobile fixes

* [components] General padding of functions
bjoerge pushed a commit that referenced this pull request Jan 30, 2018
* [components] Better dialog for mobile

* [components] Removing border on default pane

* [design] Styling button collections

* [design] Styling datepicker

* [design] General mobile fixes

* [components] Searchable select. Arrow down and handle value with no title

* [design] Mobile fixes

* [components] General padding of functions
bjoerge pushed a commit that referenced this pull request Jan 30, 2018
* [components] Better dialog for mobile

* [components] Removing border on default pane

* [design] Styling button collections

* [design] Styling datepicker

* [design] General mobile fixes

* [components] Searchable select. Arrow down and handle value with no title

* [design] Mobile fixes

* [components] General padding of functions
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.

None yet

2 participants