Skip to content
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

feat(ProfileShowcase): Drop area behaviour / design when sections expanded #13657

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

noeliaSD
Copy link
Contributor

Closes #13509

What does the PR do

  • It updates ProfileShowcasePanel with new drop area design adding shadows, changing drop areas design and sizes.
  • It adjusts ShowcaseDelegate according to design, ie: heigh and background colour on dragActive.
  • SQ / StatusDraggableListItem: Added new property colorOnDragActive that holds if background color will be changed on drag active or not.

Affected areas

Profile showcase tabs: Communities, Accounts, Collectibles and Assets

StatusQ / StatusDraggableListItem

Screenshot of functionality

Screen.Recording.2024-02-20.at.19.48.27.mov

@status-im-auto
Copy link
Member

status-im-auto commented Feb 20, 2024

Jenkins Builds

Click to see older builds (18)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ c1cc77e #1 2024-02-20 19:17:58 ~6 min macos/aarch64 🍎dmg
✔️ c1cc77e #1 2024-02-20 19:18:04 ~6 min tests/nim 📄log
✔️ c1cc77e #1 2024-02-20 19:21:55 ~10 min macos/x86_64 🍎dmg
✔️ c1cc77e #1 2024-02-20 19:22:31 ~10 min tests/ui 📄log
✔️ c1cc77e #1 2024-02-20 19:28:41 ~17 min linux/x86_64 📦tgz
✔️ c1cc77e #1 2024-02-20 19:42:33 ~30 min windows/x86_64 💿exe
✔️ d7da10a #2 2024-02-22 14:54:45 ~4 min macos/aarch64 🍎dmg
✔️ d7da10a #2 2024-02-22 14:56:46 ~6 min tests/nim 📄log
✔️ d7da10a #2 2024-02-22 14:58:13 ~8 min macos/x86_64 🍎dmg
✔️ d7da10a #2 2024-02-22 15:01:57 ~11 min tests/ui 📄log
✔️ d7da10a #2 2024-02-22 15:03:57 ~13 min linux/x86_64 📦tgz
✔️ d7da10a #2 2024-02-22 15:15:44 ~25 min windows/x86_64 💿exe
✔️ 549b26b #3 2024-02-22 15:28:52 ~6 min macos/aarch64 🍎dmg
✔️ 549b26b #3 2024-02-22 15:28:58 ~6 min tests/nim 📄log
✔️ 549b26b #3 2024-02-22 15:29:18 ~7 min macos/x86_64 🍎dmg
✔️ 549b26b #3 2024-02-22 15:32:54 ~10 min tests/ui 📄log
✔️ 549b26b #3 2024-02-22 15:37:54 ~15 min linux/x86_64 📦tgz
✔️ 549b26b #3 2024-02-22 15:45:01 ~22 min windows/x86_64 💿exe
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 9171218 #4 2024-02-23 08:23:54 ~5 min macos/aarch64 🍎dmg
✔️ 3f0e705 #5 2024-02-23 08:29:11 ~4 min macos/aarch64 🍎dmg
✔️ 3f0e705 #5 2024-02-23 08:34:22 ~9 min macos/x86_64 🍎dmg
✔️ 3f0e705 #5 2024-02-23 08:34:34 ~10 min tests/nim 📄log
✔️ 3f0e705 #5 2024-02-23 08:38:56 ~14 min tests/ui 📄log
✔️ 3f0e705 #5 2024-02-23 08:42:08 ~17 min linux/x86_64 📦tgz
✔️ 3f0e705 #5 2024-02-23 08:54:36 ~30 min windows/x86_64 💿exe

Copy link
Member

@caybro caybro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@alexjba alexjba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I have some findings below

  1. The drop area should be visible as soon as the delegate started the drag action.

Here's an example:

Screenshot 2024-02-22 at 09 47 15

App

Screenshot 2024-02-22 at 09 49 54
  1. The drop area height is much larger in the designs. In this example the pointer is below the Everyone drop area, but the Everyone drop area is active. I find this handy because you can see where you'll drop the item and improves the experience. My suggestion is to extend the drop area (without extending the button visual height) below each drop section until the Hidden header.

Design
Screenshot 2024-02-22 at 09 50 54

App
https://github.com/status-im/status-desktop/assets/47811206/d951b33a-937f-4408-9628-221e18935be3

  1. The accounts address highlight is too sensitive. Looks like the events are not properly handled or the height is too small. Sometimes it's flickering by itself without moving the pointer.
Screen.Recording.2024-02-22.at.09.49.18.mov

@noeliaSD
Copy link
Contributor Author

noeliaSD commented Feb 22, 2024

Nice work! I have some findings below

  1. The drop area should be visible as soon as the delegate started the drag action.

Here's an example:

Screenshot 2024-02-22 at 09 47 15 App Screenshot 2024-02-22 at 09 49 54 3. The drop area height is much larger in the designs. In this example the pointer is below the `Everyone` drop area, but the `Everyone` drop area is active. I find this handy because you can see where you'll drop the item and improves the experience. My suggestion is to extend the drop area (without extending the button visual height) below each drop section until the `Hidden` header.

Design Screenshot 2024-02-22 at 09 50 54

App https://github.com/status-im/status-desktop/assets/47811206/d951b33a-937f-4408-9628-221e18935be3

  1. The accounts address highlight is too sensitive. Looks like the events are not properly handled or the height is too small. Sometimes it's flickering by itself without moving the pointer.

Screen.Recording.2024-02-22.at.09.49.18.mov

Thanks for the comments!

  1. Good point, I didn't realised about that early activation. Will update it!
  2. Ok. I'll give it a thought!
  3. That's an existing issue not touched in this pr and better to review it separately. If there's no task for delegates review, I'll create a specific for this issue: The accounts address highlight is too sensitive. #13681

@micieslak
Copy link
Member

Hey, my thought regarding this point:

The drop area height is much larger in the designs. In this example the pointer is below the Everyone drop area, but the Everyone drop area is active. I find this handy because you can see where you'll drop the item and improves the experience. My suggestion is to extend the drop area (without extending the button visual height) below each drop section until the Hidden header.

I think that the key is to activate drop area whenever we the dragged item overlapping with the target section along the Y axis. Bc it seems that now mouse pointer within the target area activates it.

Copy link
Member

@micieslak micieslak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small discovery here:

Kazam_screencast_00095.webm

When moving from hidden to in showcase, the spacing below the hidden section grows and becomes quite big at some point.

@noeliaSD
Copy link
Contributor Author

One small discovery here:

Kazam_screencast_00095.webm
When moving from hidden to in showcase, the spacing below the hidden section grows and becomes quite big at some point.

Yes this is bc of the model items are hidden with heigh = 0 and the spacing is kept there. With the new model refactor this will be directly solved!

@caybro
Copy link
Member

caybro commented Feb 22, 2024

One small discovery here:
Kazam_screencast_00095.webm
When moving from hidden to in showcase, the spacing below the hidden section grows and becomes quite big at some point.

Yes this is bc of the model items are hidden with heigh = 0 and the spacing is kept there. With the new model refactor this will be directly solved!

In that case, you have to set the spacing in the listview to 0, and simulate the spacing using the delegate's inset

@noeliaSD
Copy link
Contributor Author

One small discovery here:
Kazam_screencast_00095.webm
When moving from hidden to in showcase, the spacing below the hidden section grows and becomes quite big at some point.

Yes this is bc of the model items are hidden with heigh = 0 and the spacing is kept there. With the new model refactor this will be directly solved!

In that case, you have to set the spacing in the listview to 0, and simulate the spacing using the delegate's inset

Do you think it makes sense to add more extra logic to that just to prevent this particular model usage? With the refactor, the model will just have the data needed and the listview behavior will be correct.

@caybro
Copy link
Member

caybro commented Feb 22, 2024

One small discovery here:
Kazam_screencast_00095.webm
When moving from hidden to in showcase, the spacing below the hidden section grows and becomes quite big at some point.

Yes this is bc of the model items are hidden with heigh = 0 and the spacing is kept there. With the new model refactor this will be directly solved!

In that case, you have to set the spacing in the listview to 0, and simulate the spacing using the delegate's inset

Do you think it makes sense to add more extra logic to that just to prevent this particular model usage? With the refactor, the model will just have the data needed and the listview behavior will be correct.

No need to do anything, the StatusDraggableListItem.qml already has this:

    // inset to simulate spacing
    topInset: 4
    bottomInset: 4

just set the ListView spacing: 0

@noeliaSD
Copy link
Contributor Author

One small discovery here:
Kazam_screencast_00095.webm
When moving from hidden to in showcase, the spacing below the hidden section grows and becomes quite big at some point.

Yes this is bc of the model items are hidden with heigh = 0 and the spacing is kept there. With the new model refactor this will be directly solved!

In that case, you have to set the spacing in the listview to 0, and simulate the spacing using the delegate's inset

Do you think it makes sense to add more extra logic to that just to prevent this particular model usage? With the refactor, the model will just have the data needed and the listview behavior will be correct.

No need to do anything, the StatusDraggableListItem.qml already has this:

    // inset to simulate spacing
    topInset: 4
    bottomInset: 4

just set the ListView spacing: 0

mmm, I'm not sure about this bc I've added this default height property readonly property int defaultDelegateHeight: 76 in utils (to have in only once and common) and if we relay on the inset values, then we need to calculate the specific heigh for the delegates taking into account these inset default values the component has, then from mpov it is not so clear that the value of the delegate's height comes directly from the design.

Having a proper model behavior will just solve the issue here.

@noeliaSD
Copy link
Contributor Author

First issue @alexjba pointed to solved:

Screen.Recording.2024-02-22.at.14.11.09.mov

@noeliaSD
Copy link
Contributor Author

noeliaSD commented Feb 22, 2024

  1. And after some discussions with all of you and Ben about point 2 we will go for adding an opacity to the dragged item like this:
Screen.Recording.2024-02-22.at.16.40.30.mov
  1. And as a nice to have for the future, I'll create a task for extending the droppable area like this:

image

Whenever the cursor is inside the red area, this should activate the Everyone button.

Copy link
Member

@micieslak micieslak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

…ive `

This property holds if background color will be changed on drag active or not.
…anded

- It updates `ProfileShowcasePanel` with new drop area design adding shadows, changing drop areas design and sizes.

- It adjusts `ShowcaseDelegate` according to design, ie: heigh and background colour on dragActive.

Closes #13509
@noeliaSD
Copy link
Contributor Author

Rebased and updated property name following last @micieslak suggestion.

@noeliaSD noeliaSD merged commit 1dc0362 into master Feb 23, 2024
8 checks passed
@noeliaSD noeliaSD deleted the feat/issue-13509 branch February 23, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Profile showcase] Drop area behaviour when sections expanded
5 participants