Skip to content

[UI] Define the right cursor for interface elements #13873

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.

Sign up for GitHub

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

Jump to bottom
Merged
escopecz merged 8 commits into from
Aug 26, 2024
Merged

[UI] Define the right cursor for interface elements #13873

escopecz merged 8 commits into from
Aug 26, 2024

Conversation

@andersonjeccel
Copy link
Contributor

@andersonjeccel andersonjeccel commented Jun 21, 2024 •
edited
Loading

Q A
Bug fix? (use the a.b branch) 🔴
New feature/enhancement? (use the a.x branch) 🟢
Deprecations? 🔴
BC breaks? (use the c.x branch) 🔴
Automated tests included? 🔴
Related user documentation PR URL mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #...

Description

This PR makes:

  • Page headings and other interface text use the default cursor instead of text cursor, inheriting from body
  • Everything that is in a field use the text cursor
  • All clickable things use the pointer cursor

The overall idea is to improve the interaction feedback

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Open Contacts
  3. Hover over the heading (normal cursor)
  4. Create a new Contact > Fill the fields and hover over the text inside the fields (will display the text cursor)
  5. Hover over the Saving & Close to see the pointer cursor

@andersonjeccel andersonjeccel requested review from a team, LordRembo and ricfreire June 21, 2024 14:34
@andersonjeccel andersonjeccel self-assigned this Jun 21, 2024
@andersonjeccel andersonjeccel added T1 Low difficulty to fix (issue) or test (PR) user-interface Anything related to appearance, layout, and interactivity ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging enhancement Any improvement to an existing feature or functionality labels Jun 21, 2024
LordRembo
LordRembo suggested changes Jun 25, 2024
View reviewed changes
Copy link
Contributor

@LordRembo LordRembo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is a problem from a UX point of view. If there is no explicit reason to disable a default functionality for a website, it shouldn't be done as it introduces potential friction for the end user.
We don't know how a user interacts with the various bits of text on a page. A website is not an app, they might very well have a good reason to copy headings for example.

@LordRembo
Copy link
Contributor

LordRembo commented Jun 27, 2024

@Mike-Dropsolid Would be good to have your input here as well, in light of priorities and validating the list of UI changes.

@andersonjeccel andersonjeccel marked this pull request as draft July 2, 2024 15:55
@andersonjeccel andersonjeccel marked this pull request as ready for review July 19, 2024 16:08
@codecov
Copy link

codecov bot commented Jul 19, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.53%. Comparing base (905be2c) to head (c8f1f93).
Report is 5 commits behind head on 5.x.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                5.x   #13873   +/-   ##
=========================================
  Coverage     62.53%   62.53%           
  Complexity    34362    34362           
=========================================
  Files          2260     2260           
  Lines        102762   102762           
=========================================
+ Hits          64258    64259    +1     
+ Misses        38504    38503    -1

see 1 file with indirect coverage changes

@andersonjeccel andersonjeccel force-pushed the ui-fix-selectable-interface-items branch from d3fede5 to 7cf1be2 Compare July 19, 2024 16:25
@andersonjeccel andersonjeccel changed the title [UI] Define selectable interface elements [UI] Define the right cursor for interface elements Jul 19, 2024
@andersonjeccel andersonjeccel force-pushed the ui-fix-selectable-interface-items branch from 7cf1be2 to fa2d19d Compare July 19, 2024 16:26
@andersonjeccel andersonjeccel added this to the 5.2 milestone Jul 19, 2024
@andersonjeccel andersonjeccel force-pushed the ui-fix-selectable-interface-items branch from fa2d19d to a95ee7a Compare July 19, 2024 16:29
Esthertests
Esthertests approved these changes Aug 12, 2024
View reviewed changes
Copy link

@Esthertests Esthertests left a comment

Choose a reason for hiding this comment

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

having LordRembo concern in mind, this PR is a great UX change. Nice work @andersonjeccel

@andersonjeccel
Copy link
Contributor Author

andersonjeccel commented Aug 12, 2024 •
edited
Loading

Concerns by Rembrand were addressed, I'll ask him to review again

@Esthertests Thanks for testing!

@Esthertests
Copy link

Esthertests commented Aug 12, 2024

Concerns by Rembrand were addressed, I'll ask him to review again

@Esthertests Thanks for testing!

Yes they were, I checked it also

@andersonjeccel andersonjeccel removed the ready-to-test PR's that are ready to test label Aug 14, 2024
@andersonjeccel andersonjeccel added the user-testing-passed PRs which have been successfully tested by the required number of people. label Aug 14, 2024
@andersonjeccel
Copy link
Contributor Author

andersonjeccel commented Aug 14, 2024

Warning

All Tier 1 PRs need only 1 code review and 1 user testing before able to merge. I'll update accordingly.

shinde-rahul
shinde-rahul approved these changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@shinde-rahul shinde-rahul left a comment

Choose a reason for hiding this comment

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

Code changes LGTM!!!

@shinde-rahul shinde-rahul added code-review-passed PRs which have passed code review and removed code-review-needed PR's that require a code review before merging labels Aug 23, 2024
@andersonjeccel andersonjeccel added the ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged label Aug 23, 2024
@escopecz escopecz merged commit 6db3838 into mautic:5.x Aug 26, 2024
@andersonjeccel andersonjeccel deleted the ui-fix-selectable-interface-items branch August 26, 2024 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ricfreire ricfreire Awaiting requested review from ricfreire ricfreire was automatically assigned from mautic/ux-ui-tiger-team

3 more reviewers

@LordRembo LordRembo LordRembo requested changes

@shinde-rahul shinde-rahul shinde-rahul approved these changes

@Esthertests Esthertests Esthertests approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@andersonjeccel andersonjeccel

Labels

code-review-passed PRs which have passed code review enhancement Any improvement to an existing feature or functionality ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged T1 Low difficulty to fix (issue) or test (PR) user-interface Anything related to appearance, layout, and interactivity user-testing-passed PRs which have been successfully tested by the required number of people.

Projects

Open Source Fridays
Archived in project
UX/UI Tiger Team
Status: Done

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

5 participants

@andersonjeccel @LordRembo @Esthertests @shinde-rahul @escopecz