UI: Unify uncommitted changes tab into objects #9181
Conversation
ozkatz
added
area/UI
|
like we had long time ago ;) |
|
Great initiative 😄 On the product side (havne't looked at the code yet):
|
indeed.
correct! it appears if theres one or more uncommitted change on the branch
Fixed that! clicking the uncommitted changes button toggles between showing only uncommitted changes/everything (default) - the dropdown next to it opens a dropdown with commit and rollback buttons |
|
|
||
| return ( | ||
| <Nav variant="tabs" > | ||
| <Link active={active === 'objects'} href={withRefContext(`/repositories/${repoId}/objects`)} component={NavItem}> | ||
| <DatabaseIcon/> Objects | ||
| </Link> | ||
| <Link active={active === 'changes'} href={withBranchContext(`/repositories/${repoId}/changes`)} component={NavItem}> |
| @@ -586,7 +586,7 @@ const EntryRow = ({ config, repo, reference, path, entry, onDelete, showActions | |||
| return ( | |||
| <> | |||
| <tr className={rowClass}> | |||
| {diffIndicator && <td className="diff-indicator">{diffIndicator}</td>} | |||
| <td className="diff-indicator">{diffIndicator || ""}</td> | |||
There was a problem hiding this comment.
otherwise, some lines (those that have changes) will have more td's than one that don't
| * @param repo Repository | ||
| * @param reference commitID / branch | ||
| */ | ||
| export const EmptyChangesState = ({ repo, reference }) => { |
There was a problem hiding this comment.
Probably nit, but -
This is the first func in this file.
I think it should be further down, since it makes it harder for me to understand what's this file is all about.
| <h3 className="mb-3">No Changes Yet</h3> | ||
| <p className="text-muted mb-4"> | ||
| No uncommitted changes on <code>{reference.id}</code>! | ||
| <br />Upload or modify files to see them appear here. |
There was a problem hiding this comment.
It's not advised to use
for layout (only for specific line-breaks in a text).
It gives less control on how it looks.
| ); | ||
| }; | ||
|
|
||
| export async function appendMoreResults(resultsState, prefix, afterUpdated, setAfterUpdated, setResultsState, getMore) { |
There was a problem hiding this comment.
I know it's not you who named it (well, at least not in this PR 😄),
But it's confusing - results of what?
I assume it should be something like appendMoreObjectsListResults?
There was a problem hiding this comment.
Agree - will resist the urge to do it in this PR though :)
| }) => { | ||
| const [actionError, setActionError] = useState(null); | ||
| const [internalRefresh, setInternalRefresh] = useState(true); | ||
| const [afterUpdated, setAfterUpdated] = useState(""); |
There was a problem hiding this comment.
What's afterUpdated? Why it's initialized to ""?
There was a problem hiding this comment.
it's used for pagination but terribly named. Renamed it.
There was a problem hiding this comment.
This became a huge file.
I highly suggest splitting it into smaller components.
webui/src/styles/objects/objects.css
Outdated
| } | ||
|
|
||
| /* Override action-bar button margins for button groups */ | ||
| .action-bar .btn-group .btn { |
There was a problem hiding this comment.
Why that's needed? Can't this be done using ml-0 on the object itself?
There was a problem hiding this comment.
It's not :) removed
webui/src/styles/objects/objects.css
Outdated
| } | ||
|
|
||
| .changes-action-btn { | ||
| margin: 0 !important; |
There was a problem hiding this comment.
Same here, and in tree.css.
| @@ -586,7 +586,7 @@ const EntryRow = ({ config, repo, reference, path, entry, onDelete, showActions | |||
| return ( | |||
| <> | |||
| <tr className={rowClass}> | |||
| {diffIndicator && <td className="diff-indicator">{diffIndicator}</td>} | |||
| <td className="diff-indicator">{diffIndicator || ""}</td> | |||
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This is part of @itaigilo's vision which I agree with :) some tabs in the lakeFS UI are redundant. This is step #1 - show changes when exploring a branch in the same view, a dedicated tab for it is not required and increases cognitive load.
Before:
Upload in object screen:
See the diff (and commit it) in "uncommitted changes":
After:
Commit directly from objects page: