Skip to content

refactor: remove is-focused and is-keyboardFocused #2119

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
merged 6 commits into from
Aug 29, 2023
Merged

refactor: remove is-focused and is-keyboardFocused #2119

merged 6 commits into from
Aug 29, 2023

Conversation

mlogsdon18
Copy link
Contributor

@mlogsdon18 mlogsdon18 commented Aug 22, 2023 •
edited
Loading

Description

Removes the is-focused and the is-keyboardFocused classes from the following components:

  • Asset Card
  • Floating Action Button
  • In-field Button
  • Logic Button
  • Steplist

These components do not exist in SWC so there should not be any issues on that end.

How and where has this been tested?

  1. Open the docs site for each component and ensure that the focus styling remained the same:
  • Asset card
  • Floating action button
  • In-field button
  • Logic button
  • Steplist

Regression testing

Validate:

  1. A legacy documentation page (such as accordion), including:
  • The page renders correctly
  • The page is accessible
  • The page is responsive
  1. A migrated documentation page (such as action group), including:
  • The page renders correctly
  • The page is accessible
  • The page is responsive

Screenshots

To-do list

  • I have read the contribution guidelines.

  • I have updated relevant storybook stories and templates.

  • I have tested these changes in Windows High Contrast mode.

  • If my change impacts other components, I have tested to make sure they don't break.

  • If my change impacts documentation, I have updated the documentation accordingly.

  • ✨ This pull request is ready to merge. ✨

@mlogsdon18 mlogsdon18 force-pushed the css-572-remove-focus-classes branch from 120351b to fe1f293 Compare August 22, 2023 20:41
@github-actions
Copy link
Contributor

github-actions bot commented Aug 22, 2023 •
edited
Loading

🚀 Deployed on https://pr-2119--spectrum-css.netlify.app

@github-actions github-actions bot temporarily deployed to pull request August 22, 2023 20:48 Inactive
pfulton
pfulton reviewed Aug 24, 2023
View reviewed changes
Copy link
Collaborator

@pfulton pfulton left a comment

Choose a reason for hiding this comment

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

I'm on board for this (we need to do it!), and the changes here look correct. The only other thing I'm wondering is when/how we can also prevent these classes from being applied to elements in the docs site. On quick glance, it looks like the code that handles this in enhancement.js is kind of broad and applies those selectors to a lot of elements...

@mlogsdon18
Copy link
Contributor Author

@pfulton It is currently pretty broad in order to keep the functionality working for the components that need it. It's basically a duplicate of how the focus-ring javascript was running. We could try to add checks for specific components and only run the event handlers on those? Kind of modify the code we have for Textfield a bit and I think that could work. I'll have to look into that

@pfulton
Copy link
Collaborator

pfulton commented Aug 24, 2023

@pfulton It is currently pretty broad in order to keep the functionality working for the components that need it. It's basically a duplicate of how the focus-ring javascript was running. We could try to add checks for specific components and only run the event handlers on those? Kind of modify the code we have for Textfield a bit and I think that could work. I'll have to look into that

Sounds good. We can do that as a separate ticket!

@mlogsdon18
Copy link
Contributor Author

Sounds good. We can do that as a separate ticket!

Ok I can write one up!

@mlogsdon18 mlogsdon18 force-pushed the css-572-remove-focus-classes branch from fe1f293 to 40404d5 Compare August 25, 2023 13:43
@github-actions github-actions bot temporarily deployed to pull request August 25, 2023 13:50 Inactive
@mlogsdon18 mlogsdon18 force-pushed the css-572-remove-focus-classes branch from 40404d5 to 396cb8e Compare August 25, 2023 15:12
@github-actions github-actions bot temporarily deployed to pull request August 25, 2023 15:19 Inactive
@mlogsdon18 mlogsdon18 force-pushed the css-572-remove-focus-classes branch from 396cb8e to c328fba Compare August 28, 2023 13:31
@github-actions github-actions bot temporarily deployed to pull request August 28, 2023 13:37 Inactive
@mlogsdon18 mlogsdon18 force-pushed the css-572-remove-focus-classes branch from c328fba to 441967c Compare August 29, 2023 13:17
@github-actions github-actions bot temporarily deployed to pull request August 29, 2023 13:23 Inactive
jenndiaz and others added 5 commits August 29, 2023 10:12
@jenndiaz @pfulton
@castastrophe @mlogsdon18
feat(alertdialog)!: new component (#1862)
2f31e4c 
BREAKING CHANGE: Alert variants of Dialog have been removed from Dialog and placed into their own new component, Alert Dialog

Additionally:
* feat(alertdialog): generate new component

* feat(alertdialog): use new tokens

* feat(alertdialog): adds all variants

* feat(alertdialog): adds icons

* feat(alertdialog): adds secondary button and scroll variants

* chore(alertdialog): adds variant descriptions

* fix(alertdialog): font family tokens

* chore(alertdialog): refactor icon color

* fix(alertdialog): icons on correct variants

* feat(alertdialog): underlay color

* feat(alertdialog): corner radius

* fix(alertdialog): rtl for example button

* fix(alertdialog): more rtl

* feat(alertdialog): storybook variants

* chore(alertdialog): update tokens

* chore(alertdialog): remove alerts from dialog

* fix(alertdialog): background color

* fix(alerdialog): use max height token

* fix(alertdialog): small bug fixes

* chore(alertdialog): add mods

* fix(alertdialog): aria and scrollable a11y

* chore(alertdialog): css refactor

* feat(alertdialog): work on story controls

* chore(alertdialog): improve docs

* fix(alertdialog): design feedback

* chore(alertdialog): move scale specific styling to custom vars

* chore(alertdialog): use custom props for nested components

* feat(alertdialog): adds onClick to story btns

* feat(alertdialog): adds underlay to story

* chore(alertdialog): tokens version bump

* fix(alertdialog): dialog max width

* chore(alertdialog): adds legacy token fallback

* fix(alertdialog): fallbacks for nested custom tokens

* fix(alertdialog): default for justify content

* chore(alertdialog): use % over vw

* fix(alertdialog): change default for icon sizing pass throughs

* fix(alertdialog): use initial

* fix(alertdialog): remove pass throughs for icon

* chore(alertdialog): remove space

* chore(alertdialog): update yarn.lock file

* chore(alertdialog): update yarn.lock

* chore: update mods assets for dependencies

* chore: aligning alpha releases

* chore: bump alterdialog to reflect dep changes

* chore(alertdialog): correct storybook location

---------

Co-authored-by: Patrick Fulton <pfulton@adobe.com>
Co-authored-by: castastrophe <castastrophe@users.noreply.github.com>
@mlogsdon18
refactor(assetcard): remove is-focused class
1b9fef7
@mlogsdon18
refactor(floatingactionbutton): remove is-focused class
d07641d
@mlogsdon18
refactor(infieldbutton): remove is-focused and is-keyboardFocused cla…
4ecadae 
…sses
@mlogsdon18
refactor(logicbutton): remove is-focused class and references in stor…
1b60120 
…ybook
@mlogsdon18 mlogsdon18 force-pushed the css-572-remove-focus-classes branch from 441967c to a55e9df Compare August 29, 2023 14:12
@github-actions github-actions bot temporarily deployed to pull request August 29, 2023 14:18 Inactive
@mlogsdon18 mlogsdon18 force-pushed the css-572-remove-focus-classes branch from a55e9df to 069adfb Compare August 29, 2023 14:39
@github-actions github-actions bot temporarily deployed to pull request August 29, 2023 14:46 Inactive
@pfulton pfulton merged commit 6019dd6 into main Aug 29, 2023
@pfulton pfulton deleted the css-572-remove-focus-classes branch August 29, 2023 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pfulton pfulton pfulton approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mlogsdon18 @pfulton @jenndiaz