Skip to content

Conversation

@sarutak
Copy link
Member

@sarutak sarutak commented Apr 14, 2020

What changes were proposed in this pull request?

This PR makes each id attribute for page navigations in a page unique.

PagedTable#pageNavigation returns HTML elements representing a page navigation for a paged table.
In the current implementation, the method generates an id and it's used for id attribute for a set of elements for the page navigation.
But some pages have two page navigations so there are two set of elements where corresponding elements have the same id.
For example, there are two form-completedJob-table-page id in JobsPage.

Why are the changes needed?

Each id attribute should be unique in a page.
The following is a screenshot of warning messages shown with Chrome when I visit JobsPage (Firefox doesn't show in my environment).
warning-jobspage

Does this PR introduce any user-facing change?

No.

How was this patch tested?

I added a test case for pageNavigation extended.
I also manually tested that there were no warning messages for the uniqueness in JobsPage and JobPage.

@SparkQA
Copy link

SparkQA commented Apr 14, 2020

Test build #121289 has finished for PR 28217 at commit 7bca29d.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@sarutak
Copy link
Member Author

sarutak commented Apr 14, 2020

retest this please.

@SparkQA
Copy link

SparkQA commented Apr 15, 2020

Test build #121293 has finished for PR 28217 at commit 7bca29d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

HyukjinKwon added a commit that referenced this pull request Apr 16, 2020
…nd 'dev/.rat-excludes' in BUILD autolabeller

### What changes were proposed in this pull request?

This PR excludes `ui` directly and `UI.scala` configuration file in `CORE` label, and exclude `dev/.rat-excludes` in `BUILD` label  in autolabeller. See #28218, #28217, #28214 and #28213

There are some contexts about this #28114.

The syntax is from https://git-scm.com/docs/gitignore#_pattern_format (see also https://github.com/kaelzhang/node-ignore)

### Why are the changes needed?

To label UI component properly.

### Does this PR introduce any user-facing change?

No, dev-only.

### How was this patch tested?

It uses the same syntax used for other places. I expect to see the actual results after it gets merged as it's difficult to test it out.

Closes #28228 from HyukjinKwon/SPARK-31330-followup.

Authored-by: HyukjinKwon <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @sarutak and @srowen .
I verified the duplication (before) and the uniquely generated ids (after).
Merged to master.

@dongjoon-hyun
Copy link
Member

Could you make a backporting PR to branch-3.0, @sarutak ?

@sarutak
Copy link
Member Author

sarutak commented Apr 17, 2020

@dongjoon-hyun Sure. I'll do it.

dongjoon-hyun pushed a commit that referenced this pull request Apr 17, 2020
…le to have different id attribute

### What changes were proposed in this pull request?

This PR backports #28217 to `branch-3.0`, making each id attribute for page navigations in a page unique.

PagedTable#pageNavigation returns HTML elements representing a page navigation for a paged table.
In the current implementation, the method generates an id and it's used for id attribute for a set of elements for the page navigation.
But some pages have two page navigations so there are two set of elements where corresponding elements have the same id.
For example, there are two form-completedJob-table-page id in JobsPage.

### Why are the changes needed?

Each id attribute should be unique in a page.

### Does this PR introduce any user-facing change?

No.

### How was this patch tested?

I added a test case for pageNavigation extended.
I also manually tested that there were no warning messages for the uniqueness in JobsPage and JobPage.

Closes #28246 from sarutak/backporting-SPARK-31446-branch-3.0.

Authored-by: Kousuke Saruta <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants