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.

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

Cleanup some visual state bugs #5855

Merged
merged 3 commits into from
May 6, 2022
Merged

Cleanup some visual state bugs #5855

merged 3 commits into from
May 6, 2022

Conversation

bheston
Copy link
Collaborator

@bheston bheston commented Apr 16, 2022

Pull Request

📖 Description

Quick run through some visual issues and one functional issue.
Disclosure wasn't loading because it was missing a super call.

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

@bheston bheston self-assigned this Apr 16, 2022
EisenbergEffect
EisenbergEffect approved these changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@EisenbergEffect EisenbergEffect left a comment

Choose a reason for hiding this comment

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

Seems fine on its own but will it conflict with the work m did? What's the current plan for that work? Is it part of the CLI base styles?

@bheston
Copy link
Collaborator Author

bheston commented Apr 20, 2022

Seems fine on its own but will it conflict with the work m did? What's the current plan for that work? Is it part of the CLI base styles?

"m" = Kham, meaning the work in feature/fast-component-update-visuals.

It likely doesn't directly conflict, but is something we'll probably want to carry into the visual update. I believe that branch is going to sit for a bit, then once we've established the CLI process we'll release the existing work done as a visual update. That should fit nicely into allowing people to take the changes or not.

@chrisdholt chrisdholt added the fast-components:final Issues or PR's to include in the final release of fast-components label Apr 27, 2022
@chrisdholt chrisdholt force-pushed the users/bheston/visual-bug-cleanup branch from c91c281 to ec1dabc Compare May 6, 2022 03:16
chrisdholt
chrisdholt approved these changes May 6, 2022
View reviewed changes
Copy link
Member

@chrisdholt chrisdholt left a comment

Choose a reason for hiding this comment

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

@bheston I fixed the conflicts here and rebase. There also was a build error due to the addition of the baseClass to Menu. I tested and I don't see it being applicable there (as it is in Picker). Going to merge this in once the build passes!

@chrisdholt chrisdholt force-pushed the users/bheston/visual-bug-cleanup branch 3 times, most recently from bd8b0b1 to f6e2ade Compare May 6, 2022 05:20
@github-actions
Copy link

github-actions bot commented May 6, 2022

Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-5855.centralus.azurestaticapps.net

@chrisdholt chrisdholt force-pushed the users/bheston/visual-bug-cleanup branch from f6e2ade to addaf65 Compare May 6, 2022 05:29
@github-actions
Copy link

github-actions bot commented May 6, 2022

Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-5855.centralus.azurestaticapps.net

1 similar comment
@github-actions
Copy link

github-actions bot commented May 6, 2022

Azure Static Web Apps: Your stage site is ready! Visit it here: https://purple-ocean-0b7ce3410-5855.centralus.azurestaticapps.net

@chrisdholt chrisdholt merged commit 3e8294d into master May 6, 2022
@chrisdholt chrisdholt deleted the users/bheston/visual-bug-cleanup branch May 6, 2022 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisdholt chrisdholt chrisdholt approved these changes

@EisenbergEffect EisenbergEffect EisenbergEffect approved these changes

@janechu janechu Awaiting requested review from janechu

@nicholasrice nicholasrice Awaiting requested review from nicholasrice

Assignees

@bheston bheston

Labels
fast-components:final Issues or PR's to include in the final release of fast-components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bheston @EisenbergEffect @chrisdholt