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

Allow switch between legacy table of a placeholder and data grid #5723

Closed
wants to merge 1 commit into from

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented Jan 22, 2024

Description

Add a html placeholder for default table.
Allow two switch types: 1) use advanced setting to disable legacy table 2) use toggle on discover page

After #5715 is done, we could replace the placeholder and merge it in.

Issues Resolved

#5716

Screenshot

switch-table.mov

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Add a html placeholder for default table.
Allow two switch types: 1) use advanced setting to disable legacy table
2) use toggle on discover page

Signed-off-by: Anan Z <ananzh@amazon.com>
Copy link

codecov bot commented Jan 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e83b7ee) 67.03% compared to head (6a50ff5) 67.03%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5723   +/-   ##
=======================================
  Coverage   67.03%   67.03%           
=======================================
  Files        3295     3295           
  Lines       63329    63330    +1     
  Branches    10084    10084           
=======================================
+ Hits        42451    42452    +1     
  Misses      18429    18429           
  Partials     2449     2449           
Flag Coverage Δ
Linux_1 35.24% <100.00%> (+<0.01%) ⬆️
Linux_2 55.18% <ø> (ø)
Linux_3 43.90% <100.00%> (+<0.01%) ⬆️
Linux_4 35.33% <ø> (ø)
Windows_1 35.26% <100.00%> (+<0.01%) ⬆️
Windows_2 55.15% <ø> (ø)
Windows_3 43.90% <100.00%> (-0.01%) ⬇️
Windows_4 35.33% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shanilpa
Copy link

Thanks for sharing the video in the PR @ananzh. I like that we have the setting accessible through advanced settings as well as in the application.

I do have a concern with the one in the application however - it's detached from the component that is changing (which is okay) if we provide a description on what the change could entail. Additionally, we are missing the chance to collect feedback on the new table. Here is my suggested UX for enabling new features we would like users to try.

New.features.mp4

cc @kgcreative, @ashwin-pc, @dagneyb

@ananzh
Copy link
Member Author

ananzh commented Jan 25, 2024

Close this cuz it is from a wrong branch. New one is #5739. Discussion is in the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants