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

Missing icon for Designer "other database" feature #13586

Closed
ibennetch opened this Issue Aug 14, 2017 · 6 comments

Comments

5 participants
@ibennetch
Member

ibennetch commented Aug 14, 2017

We need to pick an icon to use in the Designer for the new feature allowing users to select tables from other databases (#13455)

@ibennetch ibennetch added this to the 4.8.0 milestone Aug 14, 2017

@ibennetch ibennetch referenced this issue Aug 14, 2017

Merged

Allow designer to show tables from other DB #13455

3 of 4 tasks complete
@madhuracj

This comment has been minimized.

Show comment
Hide comment
Member

madhuracj commented Aug 29, 2017

@nijel nijel added bug ui labels Sep 4, 2017

@nijel nijel added this to To Do in Designer Sep 4, 2017

ryanml added a commit to ryanml/phpmyadmin that referenced this issue Sep 23, 2017

Fixing Issue phpmyadmin#13586, adding icon for 'Other Database' featu…
…re in Designer

Signed-off-by: Ryan Lanese <rlanese@asu.edu>

ryanml added a commit to ryanml/phpmyadmin that referenced this issue Sep 23, 2017

Fixing Issue phpmyadmin#13586, adding icon for 'Other Database' featu…
…re in Designer

Signed-off-by: Ryan Lanese <rlanese@asu.edu>

@mauriciofauth mauriciofauth self-assigned this Sep 25, 2017

@mauriciofauth mauriciofauth moved this from To Do to Done in Designer Sep 25, 2017

@ibennetch

This comment has been minimized.

Show comment
Hide comment
@ibennetch

ibennetch Sep 26, 2017

Member

Thanks for this contribution! Before we can close the issue completely, we'll still need an icon for the original theme.

Member

ibennetch commented Sep 26, 2017

Thanks for this contribution! Before we can close the issue completely, we'll still need an icon for the original theme.

@ibennetch ibennetch reopened this Sep 26, 2017

@mauriciofauth

This comment has been minimized.

Show comment
Hide comment
@mauriciofauth

mauriciofauth Sep 26, 2017

Member

Hello, @ibennetch.
I don't know why, but the Original theme doesn't have the img/pmd directory, it loads the images from the pmahomme theme.
That's why I merged the pull request, because it's working for both themes.

Member

mauriciofauth commented Sep 26, 2017

Hello, @ibennetch.
I don't know why, but the Original theme doesn't have the img/pmd directory, it loads the images from the pmahomme theme.
That's why I merged the pull request, because it's working for both themes.

@ryanml

This comment has been minimized.

Show comment
Hide comment
@ryanml

ryanml Sep 26, 2017

Contributor

@mauriciofauth @ibennetch per this conversation, since it seems to work for both, should we be fine?

I'll be happy to do something additional for the original theme so each theme can have its 'own' icon

Contributor

ryanml commented Sep 26, 2017

@mauriciofauth @ibennetch per this conversation, since it seems to work for both, should we be fine?

I'll be happy to do something additional for the original theme so each theme can have its 'own' icon

@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Dec 1, 2017

Member

AFAIK there should be fallback to default theme for non existing images, if that doesn't work something else is wrong...

Member

nijel commented Dec 1, 2017

AFAIK there should be fallback to default theme for non existing images, if that doesn't work something else is wrong...

@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Dec 1, 2017

Member

I've reverted this in 10808a2. There is really no reason to ship copies of the images. Fallback to default theme is intentional:

public function getImgPath($file = null, $fallback = null)
{
if (is_null($file)) {
return $this->img_path;
}
if (is_readable($this->img_path . $file)) {
return $this->img_path . $file;
}
if (! is_null($fallback)) {
return $this->getImgPath($fallback);
}
return './themes/' . ThemeManager::FALLBACK_THEME . '/img/' . $file;
}

Member

nijel commented Dec 1, 2017

I've reverted this in 10808a2. There is really no reason to ship copies of the images. Fallback to default theme is intentional:

public function getImgPath($file = null, $fallback = null)
{
if (is_null($file)) {
return $this->img_path;
}
if (is_readable($this->img_path . $file)) {
return $this->img_path . $file;
}
if (! is_null($fallback)) {
return $this->getImgPath($fallback);
}
return './themes/' . ThemeManager::FALLBACK_THEME . '/img/' . $file;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment