[ui-storageBrowser] Add home, refresh, edit and copy path functionalities #3996
Conversation
nidhibhatg
force-pushed
the
nidhi_sb_pb_improv
branch
from
60f30a3 to
9988eff
Compare
nidhibhatg
changed the title
nidhibhatg
requested review from
ramprasadagarwal,
JohanAhlen,
bjornalm and
tabraiz12
nidhibhatg
force-pushed
the
nidhi_sb_pb_improv
branch
from
9d92b74 to
ab7f467
Compare
desktop/core/src/desktop/js/apps/storageBrowser/StorageBrowserTab/StorageBrowserTab.tsx
Outdated
Show resolved
Hide resolved
desktop/core/src/desktop/js/apps/storageBrowser/StorageBrowserTab/StorageBrowserTab.tsx
Outdated
Show resolved
Hide resolved
desktop/core/src/desktop/js/reactComponents/PathBrowser/PathBrowser.test.tsx
Show resolved
Hide resolved
desktop/core/src/desktop/js/reactComponents/PathBrowser/PathBrowser.tsx
Outdated
Show resolved
Hide resolved
nidhibhatg
force-pushed
the
nidhi_sb_pb_improv
branch
from
009ed5d to
4496231
Compare
| display: flex; | ||
| gap: vars.$fluidx-spacing-xs; | ||
| max-width: 70%; |
There was a problem hiding this comment.
Setting 70% width with no media query might mis-align the UI element when screen size is reduces below certain point.
Can you add media query or check if UI elements are looking fine with reduced screen width?
| } | ||
| } | ||
|
|
||
| .hue-path-browser__home-btn { |
There was a problem hiding this comment.
Can we combine both home and refresh class into one?
| @@ -49,10 +52,10 @@ const StorageBrowserTab = ({ fileSystem, testId }: StorageBrowserTabProps): JSX. | |||
| urlFileSystem === fileSystem.file_system ? urlFilePath : fileSystem.user_home_directory; | |||
|
|
|||
| const [filePath, setFilePath] = useState<string>(initialFilePath); | |||
| const fileName = filePath.split('/').pop() ?? ''; | |||
| const fileName = | |||
| filePath.split('/').pop() !== '' ? filePath.split('/').pop() : filePath.split('://')[0]; | |||
There was a problem hiding this comment.
Nice fix,
Will it also cover a path ABFS://path/to/file.txt?
| @@ -20,11 +20,13 @@ $cell-height: 40px; | |||
| $table-placeholder-height: 100px; | |||
| $action-dropdown-width: 214px; | |||
| $icon-margin: 5px; | |||
| // $icon-height: 24px; | |||
There was a problem hiding this comment.
nit: remove extra code if not needed
| title={'Edit Path'} | ||
| icon={<EditIcon />} | ||
| /> | ||
| <BorderlessButton |
There was a problem hiding this comment.
I feel that the icons are enough on its own, we need not have to have border around the icons when we hover it.
Let me know your thoughts?
There was a problem hiding this comment.
If it is clickable like a button it should be a button and if we wanna use a borderless button we should follow the cuix styles for borderless buttons, which is showing the border on hover. An alternative is the LinkButton
What changes were proposed in this pull request?
How was this patch tested?
Please review Hue Contributing Guide before opening a pull request.