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: Tags ListView Page #24964

Merged
merged 58 commits into from
Sep 12, 2023
Merged

feat: Tags ListView Page #24964

merged 58 commits into from
Sep 12, 2023

Conversation

hughhhh
Copy link
Member

@hughhhh hughhhh commented Aug 11, 2023

SUMMARY

Update Tags All Entities with new designs and add Edit Modal functionality + tons of bug fixes from QA internally

Screen Shot 2023-09-11 at 8 00 02 PM

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@hughhhh hughhhh changed the title fix: remove unused file (#24946) feat: Tags Listview Page Aug 11, 2023
@hughhhh hughhhh changed the title feat: Tags Listview Page feat(draft): Tags Listview Page Aug 11, 2023
@@ -100,7 +100,6 @@ const StyledItem = styled.div<{
}
& .metadata-text {
min-width: ${TEXT_MIN_WIDTH}px;
overflow: hidden;
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that removing overflow is needed here? I wonder if it won't break anything

Copy link
Member Author

Choose a reason for hiding this comment

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

looking at the metabar in the ui it didn't effect the styling from what i could see

Copy link
Member

Choose a reason for hiding this comment

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

It's required if the text in metadata bar is too long. 1 screenshot is with overflow: hidden, second is without

image image

@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
// @ts-nocheck
Copy link
Member

Choose a reason for hiding this comment

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

ts-nocheck here too

@@ -0,0 +1,98 @@
// @ts-nocheck
Copy link
Member

Choose a reason for hiding this comment

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

ts-nocheck

* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
Copy link
Member

Choose a reason for hiding this comment

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

ts-nocheck

import React, { useMemo } from 'react';
import { ensureIsArray, styled, t } from '@superset-ui/core';
import { StringParam, useQueryParam } from 'use-query-params';
// @ts-nocheck
Copy link
Member

Choose a reason for hiding this comment

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

ts-nocheck

</AllEntitiesNav>
<div className="select-control">
{/* <div className="select-control">
Copy link
Member

Choose a reason for hiding this comment

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

Commented code

@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
Copy link
Member

Choose a reason for hiding this comment

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

ts-nocheck

addSuccessToast(`Tagged ${selected.length} items`);
})
.catch(err => {
addDangerToast('Failed to tag items');
Copy link
Member

Choose a reason for hiding this comment

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

translation

},
})
.then(({ json = {} }) => {
addSuccessToast(`Tagged ${selected.length} items`);
Copy link
Member

Choose a reason for hiding this comment

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

translation

}
>
<>
<>{`You are adding tags to the ${selected.length} entities`}</>
Copy link
Member

Choose a reason for hiding this comment

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

translation

onHide={onHide}
// @ts-ignore
onChange={tags => setTags(tags)}
placeholder="Select Tags"
Copy link
Member

Choose a reason for hiding this comment

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

translation

interface TaggableResourceOption {
const StyledModalBody = styled.div`
.ant-select-dropdown {
max-height: 100px;
Copy link
Member

Choose a reason for hiding this comment

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

theme.gridUnits

value={description}
/>
<Divider />
<FormLabel>{t('Tag Name')}</FormLabel>
Copy link
Member

Choose a reason for hiding this comment

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

'Tag name' name lowercase?

margin: 30px 0px;
}
.metadata-text {
height: 13px;
Copy link
Member

Choose a reason for hiding this comment

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

line-height: 1.4 instead of height

}`}
}
.entities {
margin: 30px 0px;
Copy link
Member

Choose a reason for hiding this comment

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

grid units

}`};
}
.header {
font-weight: 600;
Copy link
Member

Choose a reason for hiding this comment

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

font weight, font size and spacings from theme

setTag(tag);
},
(error: Response) => {
addDangerToast('Error Fetching Tagged Objects');
Copy link
Member

Choose a reason for hiding this comment

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

translation

@hughhhh hughhhh force-pushed the tags-listview branch 3 times, most recently from d00603a to fdc94d1 Compare September 12, 2023 11:34
Copy link
Member

@kgabryje kgabryje left a comment

Choose a reason for hiding this comment

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

LGTM

@hughhhh hughhhh merged commit 55ac01b into apache:master Sep 12, 2023
29 checks passed
@hughhhh hughhhh deleted the tags-listview branch September 12, 2023 12:48
@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

@mistercrunch
Copy link
Member

Nice! excited to see this merged!

jinghua-qa pushed a commit to preset-io/superset that referenced this pull request Sep 14, 2023
eschutho pushed a commit to Superset-Community-Partners/superset that referenced this pull request Sep 21, 2023
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.1.0 labels Mar 8, 2024
vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset:2023.37 size/XL 🚢 3.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants