Sync QueryView and QuerySource #4430

gabrieldutra · 2019-12-10T01:34:39Z

What type of PR is this? (check all applicable)

Other

Description

@kravets-levko this is a sync for the code, opening a PR since I was also working on some changes to simplify the Query Header component (moving style to query-page-header.less

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

- don't use fixed layout - show correct last modified by picture - order visualizations by id

gabrieldutra · 2019-12-11T14:03:38Z

@kravets-levko although the QueryView page isn't done, please give it a check when possible to sync (mainly the changes in the QueryPageHeader) and share you opinion. Link to View - Link to Source

client/app/pages/queries/components/QueryVisualizationTabs.jsx

kravets-levko · 2019-12-11T18:17:07Z

client/app/assets/less/inc/base.less

-        width: 100%;
-        margin-bottom: 5px !important;
-        display: block !important;
-      }


I noticed some difference between old and new header version: when query does not have tags, tags control should stay on the same line with query name (see screenshots). Seems this style is related to that

Seems it's better to keep markup from base branch, unless you have some better UX ideas for this component. Also, if you think that button group does not fit here - feel free to remove it.

It wraps in the older version too, just take more for it to happen 🙂. That's due to the existing conflict with .page-title { display: block }, fixed in 37c9b46

Now layout is good, but still there some differences:

existing

react

Notice: favorites control should be on the same line with query name, too much space between query name and tags, and tags should go under buttons on right side.

Yeah, in the original there are two QueryTagsControl components, one for the desktop position and another for the mobile one 😪

I usually feel css is more reliable than js when it can be used, do you have any advantages in mind?

In this case the only advantage is that tags component will be rendered only once (+markup will simplified a bit). CSS Grid is the perfect solution, but it still is not supported good enough. For JS, there is a matchMedia API which is 100% supported in all our target browsers.

BTW you meant to use a hook, right? (like this one)

It's an interesting hook to have anyway, I tried to use it once I wanted to check in JS if the browser was in Percy,

Yes, something like that 🙂

That hook is not hard to be applied in this context:

const isMobile = useMedia(['(max-width: 880px)'], [true], false);

But I prefer keeping it in css for now as I don't want to break the angular page yet (we can reconsider when cleaning angular stuff)

restyled-io · 2019-12-12T02:08:07Z

Hey there-

I'm a bot, here to let you know that some code in this PR might not
match the team's automated styling. I ran the team's auto-reformatting tools on
the files changed in this PR and found some differences. Those differences can
be seen in #4440.

Please see that Pull Request's description for more details.

client/app/pages/queries/components/QueryPageHeader.jsx

kravets-levko

Look good 👍 Feel free to merge when you're happy with the code

* Migrate Query Source View page to React: skeleton * Sync QueryView and QuerySource (#4430) * Migrate schema browser to react (#4432) * Restyle code with Prettier * Migrate Query page to React: Save changes (#4452) * Migrate query source to React: Set of updates (#4457) * Migrate Query page to React: Visualization Tabs (#4453) Co-Authored-By: Levko Kravets <[email protected]> * Migrate Query Source page to React: Visualizations area (#4463) * Migrate Query page to React: Delete visualization button (#4461) * Migrate Query Source page to React: Visualization actions (#4467) * Migrate Query pages to React: Execute query hook (#4470) * Migrate Query Source page to React: Editor area (#4468) * Migrate Query Source page to React: metadata, schedule and description blocks (#4476) * Migrate Query page to React: Cancel query execution (#4496) * Migrate Query Source page to React: refine code (#4499) * Migrate Query Source page to React: alerts (#4504) * Migrate Query Source page to React: unsaved changes alert (#4505) * Migrate Query Source to React: resizable areas (v2) (#4503) * Migrate Query page to React: Query View (#4455) Co-authored-by: Levko Kravets <[email protected]> * Switch React and Angular versions of pages (until Angular version removed) * Migrate Query pages to React: fix permissions (#4506) * Migrate Query Source page to React: don't reload when saving new query (#4507) * Migrate Query pages to React: fix tests (#4509) * Use skipParametersDirtyFlag in executeQuery * Fix: cannot fork query from Query View page * Optimize query editor: handle query text changes faster * Revert "Optimize query editor: handle query text changes faster" This reverts commit 2934e53. * Reduce debounced time to 100 * Migrate Query pages to React: cleanup (#4512) * Migrate Query pages to React: cleanup * Further cleanup * Remove unused dependencies * Fix embed pages * Attempt to fix flaky test * Cleanup: explicitly register the last Angular component * Move contents of /filters folder to /lib * Remove unnecessary import * Remove cy.wait from Parameters spec Co-authored-by: Gabriel Dutra <[email protected]> Co-authored-by: Levko Kravets <[email protected]>

Sync QueryView and QuerySource

762a388

gabrieldutra mentioned this pull request Dec 10, 2019

Migrate Query pages to React #4429

Merged

27 tasks

gabrieldutra added 3 commits December 10, 2019 12:07

Updates to layout and QueryVisualizationTabs

fae8192

- don't use fixed layout - show correct last modified by picture - order visualizations by id

Add Execute button

4030334

Remove request to DataSource.types

f80c0e1

gabrieldutra requested a review from kravets-levko December 11, 2019 12:54

Merge branch 'migrate-query-source-view-to-react' into react-query-view

e6b25e1

kravets-levko reviewed Dec 11, 2019

View reviewed changes

client/app/pages/queries/components/QueryVisualizationTabs.jsx Outdated Show resolved Hide resolved

Change currentVisualizationId prop to be number

d45357b

kravets-levko reviewed Dec 11, 2019

View reviewed changes

Fix QueryHeader wrapping behavior for mobile

37c9b46

gabrieldutra mentioned this pull request Dec 12, 2019

Fix Restyled config #4438

Merged

1 task

Merge branch 'migrate-query-source-view-to-react' into react-query-view

3f1a577

restyled-io bot mentioned this pull request Dec 12, 2019

Restyle Sync QueryView and QuerySource #4440

Closed

gabrieldutra added 2 commits December 13, 2019 14:32

Use old markup

39b1a88

Remove query-page-header.less

ef2f4c0

kravets-levko reviewed Dec 13, 2019

View reviewed changes

client/app/pages/queries/components/QueryPageHeader.jsx Outdated Show resolved Hide resolved

Remove negatie margin from query tags mobile

1230f9f

kravets-levko approved these changes Dec 13, 2019

View reviewed changes

gabrieldutra merged commit e61c628 into migrate-query-source-view-to-react Dec 13, 2019

weekly-digest bot mentioned this pull request Dec 16, 2019

Weekly Digest (9 December, 2019 - 16 December, 2019) #4451

Closed

weekly-digest bot mentioned this pull request Jan 13, 2020

Weekly Digest (6 January, 2020 - 13 January, 2020) #4539

Closed

gabrieldutra deleted the react-query-view branch January 13, 2020 17:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync QueryView and QuerySource #4430

Sync QueryView and QuerySource #4430

gabrieldutra commented Dec 10, 2019 •

edited

Loading

gabrieldutra commented Dec 11, 2019

kravets-levko Dec 11, 2019

kravets-levko Dec 11, 2019

gabrieldutra Dec 11, 2019

kravets-levko Dec 11, 2019

gabrieldutra Dec 12, 2019

gabrieldutra Dec 13, 2019

kravets-levko Dec 13, 2019

gabrieldutra Dec 13, 2019

kravets-levko Dec 13, 2019

gabrieldutra Dec 13, 2019

restyled-io bot commented Dec 12, 2019

kravets-levko left a comment •

edited

Loading

Sync QueryView and QuerySource #4430

Sync QueryView and QuerySource #4430

Conversation

gabrieldutra commented Dec 10, 2019 • edited Loading

What type of PR is this? (check all applicable)

Description

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

gabrieldutra commented Dec 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

restyled-io bot commented Dec 12, 2019

kravets-levko left a comment • edited Loading

Choose a reason for hiding this comment

gabrieldutra commented Dec 10, 2019 •

edited

Loading

kravets-levko left a comment •

edited

Loading