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

chore: update dependencies #202

Merged
merged 2 commits into from
Jul 25, 2024
Merged

chore: update dependencies #202

merged 2 commits into from
Jul 25, 2024

Conversation

MaxFrank13
Copy link
Member

@MaxFrank13 MaxFrank13 commented Jul 25, 2024

This PR addresses some work to get the Skills MFE caught up on some dependency upgrades.

Covered in this PR

  • Upgrade to Paragon v22 which includes upgrades to the Form.Autosuggest and Chip components
  • Remove redux and react-redux packages — they are not being used
  • Updates any packages prefixed with a ^ to the latest minor version
  • Adds env.config.js to .gitignore as well as provides example config file to help set up Algolia

Form.Autosuggest

The Form.Autosuggest received mostly updates in the way it's implemented in the code. However, most of these changes were made to facilitate some a11y issues that required extra work from the consumer to get the UI to be accessible. It should be accessible out-of-the-box now.

To check it out for yourself:

  • Pull down this branch
  • Run npm ci && npm start
  • Go to localhost:1992
  • Test the Autosuggest components
  • Make sure you can navigate through the form using only a keyboard

Screenshot 2024-07-25 at 9 40 45 AM

Chip

The Chip has received some UI updates that seem fine to me. The only downside is they look a little more "clickable" now than they did before. The Chip component can be clickable, but the way we are implementing them here is simply to display information.

Screenshot 2024-07-25 at 9 46 52 AM

Screenshot 2024-07-25 at 9 47 09 AM

@MaxFrank13 MaxFrank13 requested a review from a team as a code owner July 25, 2024 13:48
Copy link
Member

@justinhynes justinhynes left a comment

Choose a reason for hiding this comment

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

LGTM, but had a nitpick.

You mentioned this was a mashup of maintenance and multiple tickets; APER-3298 & APER-3330. Preferably, even if they are tiny, I like these in separate commits. That way, if ONE of the commits has a problem, we don't to roll back everything. It also helps pinpoint a problematic change.

If the change is focused on one thing and explodes, it's much easier to triage and roll back. Just food for thought.

@MaxFrank13
Copy link
Member Author

LGTM, but had a nitpick.

You mentioned this was a mashup of maintenance and multiple tickets; APER-3298 & APER-3330. Preferably, even if they are tiny, I like these in separate commits. That way, if ONE of the commits has a problem, we don't to roll back everything. It also helps pinpoint a problematic change.

If the change is focused on one thing and explodes, it's much easier to triage and roll back. Just food for thought.

Great point! My thinking here was that one of the tickets was just to remove redux and react-redux (APER-3298) which I was very confident we weren't using at all in this repo. The only update that required refactoring was the Paragon upgrade (APER-3330). But even then, your point still stands. Having small commits is better. I'll be sure to do that next time or if this one blows up, I'll revert everything and then increment the changes gradually.

@MaxFrank13 MaxFrank13 merged commit f53129d into main Jul 25, 2024
4 checks passed
@MaxFrank13 MaxFrank13 deleted the mfrank/update-dependencies branch July 25, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants