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

feat(sampledata): Route from buckets index to Data Explorer #17085

Merged
merged 7 commits into from
Mar 5, 2020

Conversation

ebb-tide
Copy link
Contributor

@ebb-tide ebb-tide commented Mar 3, 2020

@ebb-tide ebb-tide force-pushed the deniz-route-from-bucket-to-de branch from b1e76e8 to a55f597 Compare March 4, 2020 00:35
@ebb-tide ebb-tide requested a review from a team March 4, 2020 00:50
@ghost ghost requested review from drdelambre and removed request for a team March 4, 2020 00:50
Copy link
Contributor

@hoorayimhelping hoorayimhelping left a comment

Choose a reason for hiding this comment

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

This is siiiicccckkkk!! (I actually said, "sick!!" out loud when I clicked the bucket name and was taken to Explorer with it selected)

Delightful!

@@ -119,7 +128,7 @@ export default class BucketOverlayForm extends PureComponent<Props> {

private get nameHelpText(): string {
if (this.props.disableRenaming) {
return 'To rename the bucket use the RENAME button. Bucket renaming is not allowed here.'
return 'To rename bucket use the RENAME button below'
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


props.onSetActiveTimeMachine('de')
const DataExplorer: FC<Props> = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

just want to call out that switching from a PureComponent to a FunctionComponent might have performance implications since a FC always renders, while a PureComponent sometimes doesn't re-render. I don't have enough context to provide anything more helpful than that :|

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks.. I'll test it a bit..

@ebb-tide ebb-tide merged commit b1b2444 into master Mar 5, 2020
jacobmarble pushed a commit that referenced this pull request Mar 12, 2020
* feat(sampledata): Add routing to de from bucket name

* feat(sampledata): Change rename edit bucket flow

* feat(sampledata): Add onclick behavior to system buckets

* feat(sampledata): Move rename button

* feat(sampledata): Remove rename bucket button from create bucket overlay

* feat(sampledata): Fix bucket tests

* feat(sampledata): Update changelog
@jacobmarble jacobmarble deleted the deniz-route-from-bucket-to-de branch January 2, 2024 22:40
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.

None yet

2 participants