-
Notifications
You must be signed in to change notification settings - Fork 14k
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
fix: Issue #24493; Resolved report selection menu in chart and dashboard page #25157
fix: Issue #24493; Resolved report selection menu in chart and dashboard page #25157
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Menu.Item onClick={handleShowMenu}> | ||
{DropdownItemExtension ? ( | ||
<StyledDropdownItemWithIcon> | ||
<div>{t('Set up an email report')}</div> | ||
<DropdownItemExtension /> | ||
</StyledDropdownItemWithIcon> | ||
) : ( | ||
t('Set up an email report') | ||
<React.Fragment> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you probably don't need the React.Fragment here since there's a singular div as the child.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure will remove that.
Thanks for the fix @suicide11! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* fix: Issue apache#24493; Resolved report selection menu in chart and dashboard page (apache#25157) * fix: DML failures in SQL Lab (apache#25190) * fix: All values being selected in Select (apache#25202) * docs: fix wrong type in PREFERRED_DATABASES example (apache#25200) Signed-off-by: cmontemuino <1761056+cmontemuino@users.noreply.github.com> * docs: add CVEs for 2.1.1 (apache#25206) * chore: back port 2.1.1 doc changes (apache#25165) * feat(sqllab): Show sql in the current result (apache#24787) * docs(FAQ): add answer re: necessary specs, copy-edit existing answer (apache#24992) * fix: `is_select` (apache#25189) * fix: Cypress test to force mouseover (apache#25209) * fix(sqllab): Force trino client async execution (apache#24859) * fix: granularity_sqla and GENERIC_CHART_AXES (apache#25213) * chore: Convert deckgl class components to functional (apache#25177) * fix: Cypress test to force mouseover (follow-up) (apache#25223) * fix(docs): Fixing a typo in README.md (apache#25216) * chore(read_csv): remove deprecated argument (apache#25226) * chore(trino): remove unnecessary index checks (apache#25211) --------- Signed-off-by: cmontemuino <1761056+cmontemuino@users.noreply.github.com> Co-authored-by: Sandeep Patel <33354423+suicide11@users.noreply.github.com> Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Carlos M <1761056+cmontemuino@users.noreply.github.com> Co-authored-by: Daniel Vaz Gaspar <danielvazgaspar@gmail.com> Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> Co-authored-by: JUST.in DO IT <justin.park@airbnb.com> Co-authored-by: Sam Firke <sfirke@users.noreply.github.com> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Rob Moore <giftig@users.noreply.github.com> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: yousoph <sophieyou12@gmail.com> Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
SUMMARY
When we used to click on Manage Email report and then select Setup Email report, a new dialogue used to open but the submenu remains in the DOM. To remove that I have added rest props. I have checked other functionalities as well and they were working as expected.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before
After
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION