Fix: Catalog's visibility columns menu -> Empty title for Icon #4189
Conversation
Signed-off-by: Roman <ixrock@gmail.com>
ixrock
force-pushed
the
fix/catalog/icon_menu_option
branch
from
621c303 to
7441a11
Compare
|
Shouldn't that visibility just be tied to the name column? |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
…nu_option # Conflicts: # src/renderer/components/+catalog/catalog.tsx
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
| import { KubeObject } from "../../../common/k8s-api/kube-object"; | ||
| import type { CatalogEntityRegistry } from "../../api/catalog-entity-registry"; | ||
|
|
||
| const css = makeCss(styles); |
There was a problem hiding this comment.
To support css-classes auto-completion use vscode-css-modules plugin.
|
@Nokel81 @aleksfront PTALA Screen.Recording.2021-11-08.at.19.01.58.movBTW: it seems hovering "pin-icon" currently broken in latest release (it doesn't show up) |
Signed-off-by: Roman <ixrock@gmail.com>
ixrock
force-pushed
the
fix/catalog/icon_menu_option
branch
from
5ea0ca3 to
ba7f8e7
Compare
It does on master... and on my install of Also from the video, it looks a bit weird to have "Name" not be over the name. What I mean it is looks like the icon is part of that column, which looks a bit strange. |
|
|
||
| renderIcon(item: CatalogEntityItem<CatalogEntity>) { | ||
| return ( | ||
| <RenderDelay> |
There was a problem hiding this comment.
I don't think you should remove the <RenderDelay>, it was added so that the catalogue would show up faster.
There was a problem hiding this comment.
I haven't noticed big difference and in general having RenderDelay for plain icon looks weird / ugly workaround..
Solution: refactor HotbarIcon / figure out why it's slow? Most probably something wrong with material-ui thingy used inside..
There was a problem hiding this comment.
@aleksfront wdyt? I suggest to merge this already and optimize HotbarIcon (if still needed) in separated PR, maybe you can take it over if you'd like.
There was a problem hiding this comment.
I agree both. Removing RenderDelay is irrelevant to current PR, but HotbarIcon with material component should be refactored which is on todo. Let's proceed without RenderDelay.
@Nokel81 I'll add spacing in the header to align with name column text. |
Signed-off-by: Roman <ixrock@gmail.com>
ixrock
force-pushed
the
fix/catalog/icon_menu_option
branch
from
765ee44 to
c164c3b
Compare
Screen.Recording.2021-11-09.at.11.34.12.mov |
| @@ -19,43 +19,59 @@ | |||
| * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. | |||
| */ | |||
|
|
|||
| .list :global(.TableRow) { | |||
There was a problem hiding this comment.
Instead of all these changes, wouldn't it just have been easier to use showWithColumn?
There was a problem hiding this comment.
No. Because otherwise it' requires other hacks to hide text "Icon" in the header column or if we leave it empty as result we have empty menu-item for the table.
There was a problem hiding this comment.
showWithColumn hides that menu item that is the point of it.
Before:
After:
common/utils/makeCss