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 and EuiDataGrid: Add an option to enable or disable the EuiDataGrid (2.11 table) for user preference. #5716

Closed
Tracked by #5714
ananzh opened this issue Jan 19, 2024 · 34 comments · Fixed by #5789
Assignees
Labels
discover for discover reinvent v2.12.0

Comments

@ananzh
Copy link
Member

ananzh commented Jan 19, 2024

Propose to provide two ways for customer to switch between legacy table and data grid table.

  • Use a new advanced settings Use legacy table
Screenshot 2024-01-22 at 1 44 34 PM
  • Use the switch table toggle on Discover page
Screenshot 2024-01-22 at 1 40 10 PM Screenshot 2024-01-22 at 1 44 05 PM
@ashwin-pc
Copy link
Member

@ananzh Can we make both the toggle behave the same way? Right now it looks like one toggle activates the legacy table when turnd on and the other activates the new table when turned on.

Also i'd probably call it newDiscoverExperience. This lets us bundle all the changes into one instead of just the table. @shanilpa @dagneyb what do you think?

@shanilpa
Copy link

@ananzh I like that we have the setting accessible through advanced settings as well as in the application (plus 1 Ashwin's comment)

I do have additional feedback for the in app toggle. It feels 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 22, 2024

@ashwin-pc I see. Good point. Will modify the advanced settings to Use new table. Regarding newDiscoverExperience, you mean the toggle on the discover page? or in the advanced settings description? I would recommend New table since only table is updated. The side nav is updated as well as query/filter bar position.

@ananzh
Copy link
Member Author

ananzh commented Jan 22, 2024

@shanilpa @ashwin-pc Thanks for the quick feedbacks. Let me summarize what we need to change:

  • Change Use Legacy table to Use New table and modify descriptions accordingly. So default is disabled/off for both toggle and advanced settings.
  • Change New table toggle to Try new Discover features. When click it, it will pop up a helper with 1) similar descriptions as in advanced settings 2) Enable new table 3) Share feedbacks on github and link to https://github.com/opensearch-project/OpenSearch-Dashboards/issues

@dagneyb
Copy link

dagneyb commented Jan 22, 2024

@ananzh sounds good on the update to the advanced settings phrasing to align with the in-line selector. I agree with @shanilpa 's feedback that we should bring the toggle closer on the UI to the element it is impacting. I'm not fully aligned with the UI @shanilpa proposed in the video above however. The goal should be that we dont have multiple features being enabled/disabled at a given time, we should keep it really simple for each release with 2 options, not giving an on/off switch for all features. With that said, could we align on a toggle experience that makes it more obvious in this release that it controls the table, and if we use a toggle in a future release to try out a new Discover experience with multiple changes, that toggle would go back to the high level location proposed in the original issue here. Thoughts?

@ananzh
Copy link
Member Author

ananzh commented Jan 22, 2024

I think Enable new table is the only option right? Enable new feature is just something we might expand in the future right?

@shanilpa and @dagneyb could we make a decision whether we want to use a single toggle or something like the mock with only one Enable new table?

@shanilpa
Copy link

@dagneyb we can have a checkbox in the modal that says enable all new features so users can select all without having to click individually. I do think the granular control has several benefits:

  1. User transparency on what is going to change and to what (especially if we bundle changes in Discover)
  2. Allows users to pick and chose what functionality they would like based on their workflows (it isn't all or nothing)
  3. Allows deep links if users want to provide feedback on a specific change

For this specific change since it is only the table if we can have the control closer to the table like in the original issue I think that works: #5555 (comment) (plus it sounds like users are okay with the placement of the toggle there)

@ananzh
Copy link
Member Author

ananzh commented Jan 23, 2024

Since table is only switchable component and given a tight timeline to mitigate users pain, we will keep a simple toggle to avoid any complexity and confusion. Meanwhile, we already have an established feedback mechanism (accessible via the ? icon) that our customers are familiar with. This feature has been a part of OSD from the start. We won't introduce a separate feedback channel specifically for the new Discover which might lead to redundancy. Based on the discussion, I would like to confirm the changes

  • Keep toggle but change New table to New Discover Experience (@shanilpa Is this too long? Would like to have ur confirmation.) in the top nav as shown in the above.
  • Add a dismissable notice (@shanilpa which way you prefer?)
    • Add notive like we did in 2.10 that lets the user know about the toggle and also points to them that they could click ? to provide feedback.
    • Add a notice like we did in 2.10? because customer should be familiar with our feedback loop.
  • Change Use Legacy table to Use New table and modify descriptions accordingly in the advanced settings. So default is disabled/off for both toggle and advanced settings.

@shanilpa
Copy link

Since we need this on a short turn around I am okay with the toggle. We can validate if this is the best implementation long term for future releases.

  • We can change New table to New Discover experience @vagimeli does this work from a copy perspective?
  • We can add the dismissible notice. The ? feedback form is for general feedback and has questions that are too general for what we need. I can see users get frustrated with the form and not leave feedback. I think we should deep link to a specific Github issue or forum post so it's easy for users to provide feedback. @ananzh let's work with @vagimeli to get the copy for this banner as well.

@wbeckler
Copy link

wbeckler commented Jan 23, 2024

What do you think about making this setting browser-specific by way of Josh's new browser-based theme settings? We would put the switch next to the table and wire it up to the interface that Josh created in #5652.

@dagneyb
Copy link

dagneyb commented Jan 23, 2024

@wbeckler This should definitely be browser-specific if we can. @ananzh is that possible in time?

@vagimeli
Copy link
Contributor

From a user POV, the button style is more eye-catching than the toggle. I suggest revisiting the design style, especially when we release more robust features.

@kgcreative
Copy link
Member

What do you think about making this setting browser-specific by way of Josh's new browser-based theme settings? We would put the switch next to the table and wire it up to the interface that Josh created in #5652.

This should be a per-user setting in the fullness of time, so individual users can decide for themselves which experience they want to use.

@shanilpa
Copy link

@ananzh here is the updated prototype with the toggle.
@vagimeli has reviewed the copy as well.

New.featuresc2.0.mp4

@vagimeli
Copy link
Contributor

@ananzh here is the updated prototype with the toggle. @vagimeli has reviewed the copy as well.

New.featuresc2.0.mp4

LGTM!

@ananzh
Copy link
Member Author

ananzh commented Jan 25, 2024

@wbeckler will check Josh's PR and update.
@vagimeli and @shanilpa I have made the changes based on the requests, pls help me to verify. The recording including the new table feedback panel in both light and dark mode and the changes in advanced settings.

add_feedback.mov

@shanilpa
Copy link

This looks good @ananzh. @kgcreative @dagneyb?

Couple questions and feedback depending on the answer:

  1. Is the survey opening in a new tab? If not it should.
  2. For the survey we should chat with Aparna to get a specific one for this feature. We shouldn't use the general Dashboards survey. @ashwin-pc did this in the past and should be able to help you out on how to get this. If not lmk and I can see if we can get that.

@ananzh
Copy link
Member Author

ananzh commented Jan 26, 2024

@shanilpa

Will update

  1. Allow the survey opening in a new tab.
  2. Will ask Aparna for a special one for feedbacks.

@ananzh
Copy link
Member Author

ananzh commented Jan 27, 2024

Update based on request to use local storage and remove advanced settings to avoid any risk for customers. Now the setting is in the browser storage:

  1. open a new tab with same browser will still see the setting
  2. open another browser will have no setting
  3. clean the storage will remove setting (tested with chrome window and then incognito window)
local-storage.mov

@kgcreative
Copy link
Member

Thanks all!

A few small updates - Users expect that a toggle will switch the interface immediately, so having a full page refresh based on a toggle breaks that expectation. Let's align to using a (small) empty button with icon left instead in stead of the toggle:

image

Then, for the go-back experience, let's update the modal to the following:

image

Title: Share your thoughts on the latest Discover features
Body: Help drive future improvements by providing feedback about your experience.
Primary Action: Turn off new Discover

@ashwin-pc ashwin-pc linked a pull request Feb 5, 2024 that will close this issue
7 tasks
@ashwin-pc ashwin-pc removed a link to a pull request Feb 5, 2024
7 tasks
@ananzh
Copy link
Member Author

ananzh commented Feb 7, 2024

Resolved in #5789

@ananzh ananzh closed this as completed Feb 7, 2024
@ohTHATaaronbrown
Copy link

ohTHATaaronbrown commented Feb 23, 2024

So, this got implemented in 2.12. Great. I can't use that because my OS back-end version is not 2.12. That makes this toggle completely unreachable for me. While this grid may solve technical debt issues for you, it makes the discover app almost completely unusable for my main use of opensearch dashboards, which is searching and reviewing logs. Now the log message is just one tiny, dumb, column among many tiny dumb columns, all of which are the same width and height.

@wbeckler
Copy link

wbeckler commented Mar 6, 2024

Are you self-hosting or are you using AWS's managed service? This is getting fixed as a patch to v2.11 on the managed service.

@ohTHATaaronbrown
Copy link

I self-host OSD against a managed OS cluster, because the built-in OSD makes all kinds of assumptions about security and access that don't fit my use case. This should be back-ported to a 2.11 patch, period. For all release paths of 2.11. The new table just breaks huge swaths of the functionality of the app.

@wbeckler wbeckler reopened this Mar 9, 2024
@wbeckler
Copy link

wbeckler commented Mar 9, 2024

@ananzh @ashwin-pc thoughts?

@ashwin-pc
Copy link
Member

@ohTHATaaronbrown for your usecase you can add opensearch.ignoreVersionMismatch: true in your yaml config or pass the --opensearch.ignoreVersionMismatch=true flag in the CLI if thats how you run it to run OSD 2.12 against OS 2.11. This should not cause any issues since 2.12 should be backwards compatible with any 2.x version of OpenSearch

@ohTHATaaronbrown
Copy link

ohTHATaaronbrown commented Mar 12, 2024

ValidationError: [config validation of [opensearch].ignoreVersionMismatch]: "ignoreVersionMismatch" can only be set to true in development mode

Wha? What is development mode? Why is this made so difficult? Why is OSD even version-locked to the minor version in the first place?

Please explain why this can't be made to be in v2.11 for all release paths, like it should be. You're patching 2.11 for the AWS managed version, so it's clearly possible. Why force an upgrade that requires a mode hack to fix something that breaks a good portion of the app's function?

Version locking + massive UI changes with no feature switch or rollback capability is a really bad combination of things.

In the future, y'all might seriously consider always putting major changes to the UI behind a feature switch so that you don't sow chaos in the community with sudden removal of features that used to be present. Don't just move cheese and then act surprised when it makes people upset.

@ashwin-pc
Copy link
Member

@ohTHATaaronbrown Oh I wasn't aware that Dev mode was required to enable that. Yes that is silly and a restriction that we inherited from Kibana when we forked. @AMoo-Miki should have a way to unblock you temporarily until 2.13 which is also coming to AWS soon.

Honesty we had a tough call to make on what releases we could support for this fix since each release is quite time consuming, thats why for this toggle and the two user groups we targeted were Open Source users (using 2.12) and AWS users (using 2.11 since they would not get 2.12). I didn't anticipate anyone who isnt a dev from using a 2.11 AWS version with a 2.12 OpenSource in a production environment since thats not something that we've publicly supported even though its technically possible and probably safe to do.

Given that the 2.13 release is already underway I'll let @AMoo-Miki comment on how to get around the dev mode restriction temporarily until then.

@ohTHATaaronbrown
Copy link

I'm a troublemaker by nature. Sorry about that :)

@AMoo-Miki
Copy link
Collaborator

AMoo-Miki commented Apr 22, 2024

@ohTHATaaronbrown you are my favorite kind of user!

The upcoming version of OSD, 2.14.0, will allow you to use opensearch.ignoreVersionMismatch: true in the config file without forcing you into dev mode.

However, if you want to do it right now with 2.13.0:

  1. open src/core/server/opensearch/opensearch_config.js in your OSD installation
  2. locate the line that contains "ignoreVersionMismatch" can only be set to true in development mode
  3. delete that line from start to end.
  4. start/restart OSD.

@AMoo-Miki
Copy link
Collaborator

@ananzh can this be closed?

@chadmyers
Copy link

@AMoo-Miki I just saw the release notes for Dashboards and saw your pull request. Thank you!

@ohTHATaaronbrown
Copy link

@AMoo-Miki Thank you!

@ashwin-pc
Copy link
Member

Closing this issue since 2.14 has been released and you no longer need to be in dev mode to ignore the version mismatch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discover for discover reinvent v2.12.0
Projects
None yet