feat(custom-views): Add styles to draggable tabs #73543
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
Scope: Frontend
MichaelSun48
commented
Jun 28, 2024
MichaelSun48
force-pushed
the
msun/styledDraggableTabs
branch
from
5c7f196 to
7b68301
Compare
roggenkemper
reviewed
Jul 2, 2024
MichaelSun48
force-pushed
the
msun/styledDraggableTabs
branch
from
f089ee4 to
412a753
Compare
malwilley
reviewed
Jul 2, 2024
| Add View | ||
| </AddViewButton> | ||
| {isTempTabVisible && <TabDivider />} | ||
| {isTempTabVisible && tempTab && ( |
Member
There was a problem hiding this comment.
I think it makes more sense to render the "Add view" button and temp tab outside of this component. They are separate from the list tabs and it will be easier to make this component more generic that way.
I also think if you are able to remove the implementation-specific details here, we should be ready to merge this component into master.
| </Reorder.Item> | ||
| ))} | ||
| <AddViewButton borderless size="zero" onClick={() => setIsTempTabVisible(true)}> | ||
| <IconAdd size="xs" style={{margin: '2 4 2 2'}} /> |
Member
There was a problem hiding this comment.
I this margin necessary? The button styles should have centered it correctly I'd . If they are necessary it should be in a styled component.
…4679) Fixes #74156 so the toolbar will be totally hidden when submitting feedback / taking a screenshot. https://github.com/user-attachments/assets/6f733424-5520-495a-b06f-b17c6be50117 --------- Co-authored-by: Ryan Albrecht <ryan.albrecht@sentry.io>
Adds `disallowWildcard` config option. Will mark invalid tokens and free text as red and display a tooltip explaining the issue.
…lag (#74700) Created migration to add bit flag to Organization. Will need to follow this up with a migration to add the flag so Hybrid Cloud services can handle syncing. **Glossary** **Data secrecy mode:** Disallows any kind of superuser access into an organization **Enable/Disable Data secrecy mode:** Persistently enable/disable data secrecy for an organization **Waive Data secrecy mode:** Temporarily disable data secrecy for an organizations **Reinstate Data secrecy mode:** Re-enable data secrecy after a temporary waiver This flag handles the enable/disable function. [spec](https://www.notion.so/sentry/Superuser-Data-Secrecy-Mode-b9f7fdfd8b564615ae1f91d3d981bc1a)
…ed (#74771) Add reject_on_worker_lost flag to re-queue the message if the worker is killed
If you have never sent a span with ai.model_id, the span metric ai.total_cost will never be extracted, and then the indexer will throw an error if you try to look for it. This separates the "ai.total_cost" and "ai.total_tokens.used" table entries --------- Co-authored-by: George Gritsouk <989898+gggritso@users.noreply.github.com> Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
- closes getsentry/team-replay#450 - this is the branch off from #74540, the experimental refactor of `replayerStepper` - TLDR: memoize the `replayerStepper` calls so that we remember the results and don't have to reload the data every time. **current state of things**: we have distinct `replayerStepper` calls in the breadcrumbs + memory tabs. in total, we could be calling it 3 times because it's used by (1) the breadcrumbs tab to render the HTML snippet, (2) for hydration error text diffs, (3) and the DOM nodes chart. **the plan**: improve things by calling `replayerStepper` once (or at least less) and memoize the results. side note: it was hard to combine the hydration error text diff `replayerStepper` call with the others due to the types, so we had to keep that call separate. **the question**: is it faster to run one `replayerStepper` instance, and collect all the data at one time (iterate over the frames exactly once)? this means the speed of any tab will always be limited by the slower call, since we're going through ALL the data at once, even some that we might not need yet. like this: https://github.com/getsentry/sentry/blob/70c90544f39dcc3d87dea8c85c0a7669c9585403/static/app/utils/replays/replayReader.tsx#L363-L366 or is it better to continue to have 2 stepper instances that iterate over 2 different sets of frames (breadcrumbs loops over hundreds of frames, and DOM node count iterates over thousands, which means the speeds could be drastically different). in this situation, each tab would handle their own array of necessary frames. **the experiments**: the first PR (#74540) tried to explore a refactor where `replayerStepper` is called once for the two DOM node actions (counting, for the memory chart, and extracting, for the breadcrumbs HTML). changes were made to the stepper so it could accept a list of callbacks, and return all types of data in one shot, so we only need one hidden `replayer` instance on the page for the DOM node functions. unscientifically, it seemed like loading breadcrumbs + memory took the same amount of time as loading the memory chart. (see video below -- as soon as the breadcrumbs tab is done loading, the memory tab is already done). https://github.com/user-attachments/assets/2c882bcb-c8df-409c-8e2a-69d75c279735 this makes sense because we’re iterating over thousands of frames for the DOM nodes chart, so that’s the bottleneck. this means the breadcrumbs tab loads slower. **this PR**: compared to that is the approach where we have 1 stepper instance for breadcrumbs, and one for DOM node count, each iterating over their own lists (which is what i'm doing in this PR). this approach showed breadcrumb data on the screen about 2x as fast as the approach above. therefore 2 stepper instances on the screen is better for users, especially since breadcrumbs tab is more popular than the memory tab. the video below demonstrates the memoization in action. once the breadcrumbs or memory tabs are loaded, switching back to them does not cause re-loading, because the results are cached. notice that the breadcrumbs tab loading is not dependent on the loading of the DOM tab, unlike the video above where the breadcrumbs tab has to "wait" for the memory tab. (i'm also on slower wifi in this clip below) https://github.com/user-attachments/assets/be71add1-63b5-4308-9d26-356d544980ef
Fields in RPC models that begin with underscores are automatically excluded: https://docs.pydantic.dev/1.10/usage/models/#automatically-excluded-attributes To avoid this, we need to mark them with [`PrivateAttr()`](https://docs.pydantic.dev/1.10/usage/models/#private-model-attributes). Otherwise, `RpcOrganization.default_owner_id` will always return `None`. This is needed for getsentry/getsentry#14709.
github-actions
bot
added
the
Scope: Backend
Contributor
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
MichaelSun48
added a commit
that referenced
this pull request
Jul 26, 2024
This PR creates the `<DraggableTabs/>` component along with many supporting components. It utilizes the new tab variant [recently introduced](#74867), and adds support for reordering them via dragging (implemented w/ Framer motion). This PR contains a subset of changes from a previous [PR](#73543) that contained a lot of new changes to the draggable tabs component, and it will be the first in a series of PRs that break up that PR into smaller chunks. This PR _should_ be the largest of them all, since it creates all of the draggable tabs components. I have tried to pare the old PR down to _only_ contain the changes necessary to get the draggable tabs to be actually draggable (using Framer motion), but please feel free to comment on anything that looks like it may be an artifact of these changes. Known issues in this current implementation that will be fixed in subsequent PRs: - There are no tests for this component - Overflow tabs interact with the tab dividers very weirdly - Tab names wrap very weirdly https://github.com/user-attachments/assets/329fe197-b6af-47af-8faa-4d34ef65181f
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The purpose of this PR is to style the draggable tabs component to match the mockups, and to add some basic functionality to make it easier to connect the backend endpoints in the near future. In addition to changes made to the draggable tabs component, the a new prop,
isNewVarianthas been added to the existing<Tabs>component, which controls whether to use the new variant's styles or the existing styles.This also implements some basic functionality such as:
+ Add ViewbuttonVisual things that are still incomplete: