Skip to content

Conversation

@somebody1234
Copy link
Contributor

@somebody1234 somebody1234 commented Apr 9, 2025

Pull Request Description

  • Close https://github.com/enso-org/cloud-v2/issues/1907
    • ℹ️ Remove "Labels" panel on left sidebar, and associated functionality:
      • "New Label" modal
      • Labels drag handling
        • Temporary visual label state for rows
    • Consistent text for buttons disabled due to being Cloud-exclusive (both on the Drive Bar and Asset Panel on the right)
    • ghost-icon Button variant so that icons with ghost variant are visible again
      • This affects both the "Labels" column, and the "Keyboard Shortcuts" table in the Settings.
    • Improve "x items copied"/"x items cut" indicator in Drive Bar
      • Possibly temporary, may be replaced later. This is a simple visual change though
    • Allow cut assets to be selected
      • Assets being deleted/restored (still) cannot be selected, however.
    • (Reverted) Move menu arrow to the right of "Community" to below the button
      • Center "Community" menu below the button
    • Remove extra spacing for (Local) categories between right side of label and "Remove from Sidebar" button
    • Make "Keyboard Shortcuts" table in the Settings be full width
    • Remove background dimming from context menus (and add shadow) for consistency
    • Prevent button bar in "Members" section in Settings from (incorrectly) growing vertically
    • Hide "hidden columns" tray when all columns are visible
      • If not hidden, it will have a background that will cover the table header when scrolled in such a way that they overlap
      • Close DevTools when interacting outside the popover
    • Remove (incorrect) slight sideways movement on Drive Table
      • This includes both the assets list and the dropzone (both move separately)
      • Be sure to check for (no) sideways movement on both sides (fully scrolled to the left, and fully scrolled to the right)

Important Notes

None

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

@somebody1234
Copy link
Contributor Author

adding @jdunkerley for review of the text changes. remember that they can easily be reverted/changed so feedback is more than welcome

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

🧪 Storybook is successfully deployed!

📊 Dashboard:

@PabloBuchu
Copy link
Contributor

  1. Labels category. Lets remove the left labels picker from both local and cloud (keep the column and dialog selector)
  2. Lets revert the changes with community arrow

@somebody1234
Copy link
Contributor Author

somebody1234 commented Apr 10, 2025

@PabloBuchu should be addressed now. just a heads up that removing the "labels" panel in the sidebar means a lot of other stuff is now removed (as mentioned in edited original post):

  • tests for labels sidebar panel (of course)
  • "add label" dialog
    • "manage labels" dialog in the "labels" column now lets you create new labels with arbitrary names
      • it also now lets you delete labels - otherwise there is no way
  • drag and drop for labels, and drag previews on rows
    • this makes up a lot of the removed code (a lot of the other removed code is removing components wholesale)

we may want to consider having a label management page in settings but IMO that is out of scope here

ℹ️ basically the goal is to not lose functionality

@PabloBuchu
Copy link
Contributor

The label selector is doing crazy things.... Also please remove the scrollbar in the middle. If needed it should be to the right of the modal. Delete icon should also be aligned to the right

Screen.Recording.2025-04-11.at.14.38.56.mp4

@PabloBuchu
Copy link
Contributor

@somebody1234 You said it was fixed...

Screen.Recording.2025-04-14.at.10.03.36.mp4

@PabloBuchu
Copy link
Contributor

Please take time to fix it properly, its not required for the release. Obviously the list directory requests are overwriting the selected labels

Screen.Recording.2025-04-14.at.10.28.47.mp4

@somebody1234
Copy link
Contributor Author

list directory requests are overwriting the selected labels

my bad! i think this was introuduced before this PR actually, a useEffect to sync up state with the actual asset state so that newly created labels appear as selected

(but of course, i realize now that that's not required, the same can be achieved by manually updating the list of labels.)
(so should be fixed now)

@PabloBuchu
Copy link
Contributor

I guess one last thing is to add ... if there is too much labels

Screenshot 2025-04-22 at 10 26 22

@somebody1234
Copy link
Contributor Author

@PabloBuchu i think maybe that one should be part of a new issue, that one needs a little thought i think. for example we can't just put it in a tooltip, and i don't believe there is an easy/neat way to do this in CSS

@somebody1234 somebody1234 added the CI: Ready to merge This PR is eligible for automatic merge label Apr 29, 2025
@mergify mergify bot merged commit a97eecf into develop Apr 29, 2025
63 of 66 checks passed
@mergify mergify bot deleted the wip/sb/minor-visual-adjustments branch April 29, 2025 09:13
@jdunkerley jdunkerley added this to the 2025-Q2 Release milestone Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

--bug Type: bug CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge g-dashboard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants